Support Js.Date.t

[joseferben]

Hey @ryb73!

First of all thanks for this wonderful ppx, it helped to get rid of quite some boilerplate code.

What would be really helpful if ppx_decode shipped with Js.Date.t decoder, even though JSON has no support for dates. Something like https://mlms13.github.io/bs-decode/docs/decoding-simple-values does, some sort of best effort new Date call on the string that is supposed to be a date.

Thanks,
Josef

[mrmurphy]

I agree, it'd be nice to have some default. I've implemented multiple versions of my own Decco codecs for date, since sometimes I have to work with dates as ISO strings, and other times I work with them as number timestamps. But it would be nice to have Date available for Greenfield projects, and have it work with ISO strings by default.

[ryb73]

Interesting. I'm not sure yet about supporting Js.Date.t by default, for the reasons @mrmurphy mentioned, but it would make sense at least to add codecs that can be used with [@decco.codec]

[mrmurphy]

@ryb73 I think that's a good solution!

[tsnobip]

hi @ryb73 I'd like to make a PR to add the codecs for Js.Date.t (one for ISO date string and one for timestamp), are there instructions on how to contribute?
I've forked, cloned, yarn installed, yarn postinstalled, yarn build-ppx, but when I enter yarn build-lib or yarn watch I get these errors :

yarn run v1.17.3
$ bsb -clean-world -make-world
Cleaning... 6 files.
Cleaning... 0 files.
[5/5] Building src/jest.cmj
[1/9] Building src/Codecs.mlast
env: node\r: No such file or directory

  We've found a bug for you!
  /Users/paul/code/ppx_decco/src/Codecs.re
  
  There's been an error running a preprocessor before the compilation of a file.
  This was the command:
  
  /Users/paul/code/ppx_decco/./ppx/ppx_decco.sh '/var/folders/_9/dh8gfmg13hz95cnhsgw4x_8m0000gn/T/camlppxe04fc7' '/var/folders/_9/dh8gfmg13hz95cnhsgw4x_8m0000gn/T/camlppx37c7b2'
  
[2/9] Building src/Decco.mlast
env: node\r: No such file or directory

  We've found a bug for you!
  /Users/paul/code/ppx_decco/src/Decco.re
  
  There's been an error running a preprocessor before the compilation of a file.
  This was the command:
  
  /Users/paul/code/ppx_decco/./ppx/ppx_decco.sh '/var/folders/_9/dh8gfmg13hz95cnhsgw4x_8m0000gn/T/camlppx04fd7e' '/var/folders/_9/dh8gfmg13hz95cnhsgw4x_8m0000gn/T/camlppx1b42a9'
  
[3/9] Building __tests__/test.mlast
env: node\r: No such file or directory

  We've found a bug for you!
  /Users/paul/code/ppx_decco/__tests__/test.re
  
  There's been an error running a preprocessor before the compilation of a file.
  This was the command:
  
  /Users/paul/code/ppx_decco/./ppx/ppx_decco.sh '/var/folders/_9/dh8gfmg13hz95cnhsgw4x_8m0000gn/T/camlppx58b130' '/var/folders/_9/dh8gfmg13hz95cnhsgw4x_8m0000gn/T/camlppx20671f'
  
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[tsnobip]

great lib by the way :)

[ryb73]

Thanks! Not at my computer right now, I'll take a closer look later. What OS are you using? It sounds like yarn build-ppx was successful but yarn build was not?

…

On Wed, Oct 23, 2019 at 7:57 PM tsnobip ***@***.***> wrote: great lib by the way :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHGK5GFMHXKJOXFU5LQSFTQQDQHRANCNFSM4ITPCGXA> .

[tsnobip]

Yes yarn build-ppx was successful unlike yarn build-lib. I'm using macOS Mojave.

[tsnobip]

it's my first time building native, do I have to install opam, make a switch to 4.02 etc?

[ryb73]

No, opam shouldn't be needed as esy will handle the installation of ocaml. What if you try running something like this?

$ echo "let _ = 1" > test.ml                                                                                                                                         master 
$ ./ppx/_esy/default/build/default/.ppx/ppx_decco/ppx.exe test.ml

[tsnobip]

when I do that it prints :

let _ = 1

[ryb73]

Ok, so that's working. What if you define a type in that ml file with a @@decco attribute? I'm trying to isolate the failure I'm on the discord as rpbiwer if you're interested in continuing this discussion in chat

…

On Thu, Oct 24, 2019 at 4:40 PM tsnobip ***@***.***> wrote: when I do that it displays : let _ = 1 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHGK5HSPWPMWBRCOI6S6W3QQIB3TANCNFSM4ITPCGXA> .

[tsnobip]

OK I found the issue, it was just an end of line issue in ./ppx/ppx_decco.sh, converting it to LF made it work. Might be worth another PR to make it cross-platform using a .gitattributes :)