rescript-labs/decco

Support Js.Date.t

Opened this issue · 12 comments

joseferben commented

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 commented

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.

  • 👍1
ryb73 commented

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]

  • 👍2
mrmurphy commented

@ryb73 I think that's a good solution!

tsnobip commented

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 commented

great lib by the way :)

ryb73 commented
tsnobip commented

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

tsnobip commented

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

ryb73 commented

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 commented

when I do that it prints :

let _ = 1
ryb73 commented
tsnobip commented

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 :)

  • 👍1