search

sikanhe / reason-nodejs

Node bindings for Reason and Bucklescript
MIT License
100 stars 18 forks source link

How to add `utf8` read option for the `Fs.readFileSync`? #53

Open wontheone1 opened 3 years ago

wontheone1 commented 3 years ago

It seems to me, from the way readFileSync binding is done, I cannot provide "utf8" option yet. Could someone confirm it?

readFileOptions defined as, https://github.com/sikanhe/reason-nodejs/blob/master/src/Fs.re#L203-L204

Flag defined as, https://github.com/sikanhe/reason-nodejs/blob/master/src/Fs.re#L140-L189

I need to give utf8 to read text files properly as in NodeJs example: https://nodejs.dev/learn/reading-files-with-nodejs

Looks to me, I should add something like the following to the definition of Flag.

...
  [@bs.inline "utf8"]
  let utf8: t;
...
  [@bs.inline "utf8"]
  let utf8 = "utf8";
...

But I am not 100 % sure (because I am Reason beginner still). Should I make PR with the above code change or could the library maintainers add utf8 flag for readFileSync function?

Thank you :)

wontheone1 commented 3 years ago

@austindd Could you take a look at the issue (seemed like you were one of the more active maintainers)? Thank you!

TheSpyder commented 3 years ago

The trick was to add encoding as a string param to readFileOptions. I've included this change in TheSpyder/rescript-nodejs#1 (I just published v14)

Note that I separated readFileSync from readFileSyncWith to match the way other filesystem APIs are modelled - this is a breaking change but seemed like a good time to do it.