use ?? and fs.promises

[jimmywarting]

readFile and readFileSync didn't exactly do the same thing. awaiting the readFile should have been inside of the try catch block and respect the throws options. (reading a file might fail)

i also moved some things around for better typing, overriding some let variable with a different type spooks sometimes.

---

I also wanted to remove graceful-fs and universalify and be completely sync or async (and remove callback method)
but thought i would ask first

Where also thinking of switching this from cjs to ESM also if that is alright.

[zardoy]

I'm pretty sure using ?? isn't backward-compatible, so engines should be added as well. Another way is to use preprocessors for the source. Consider migrating to TypeScript if you want move package to ESM. Another way is a natural migration. Have a look at using https://github.com/sindresorhus/load-json-file / https://github.com/sindresorhus/write-json-file

[zardoy]

I'm sure this is used for checking for the lint issues, since it's referenced from test script. As far as I understand it would break dead CI integration.

[RyanZim]

Yeah, lint shouldn't run --fix

[RyanZim]

readFile and readFileSync didn't exactly do the same thing. awaiting the readFile should have been inside of the try catch block and respect the throws options. (reading a file might fail)

This is a known inconsistency. I'd like to fix it, but I'm not convinced moving the readFile to have an option to completely silence the error is the correct solution. See jprichardson/node-fs-extra#542 (comment) for details.

I also wanted to remove graceful-fs and universalify and be completely sync or async (and remove callback method)
but thought i would ask first

In support of removing universalify; let's keep the optional graceful-fs as-is for now.

Where also thinking of switching this from cjs to ESM also if that is alright.

We should support both CJS & ESM; which most likely means keeping the main module CJS, and adding an ESM wrapper. I'd prefer to do this myself, if you don't mind.

[RyanZim]

Also, please rebase, update GitHub Actions and CircleCI configs to test on the correct versions, and add engines.

[zardoy]

@RyanZim but what do you think about TS migration? Are familiar with tsc? By using it we can generate both cjs and esm from one codebase (I hope it'd work for esm).

And why CircleCI should be added if you've already created GitHub workflow?

[RyanZim]

@zardoy I don't know TypeScript, so it isn't really an option, since I have to be able to maintain the codebase.

Sorry, GitHub Actions and AppVeyor config, not CircleCI 🤦‍♂️

[zardoy]

AppVeyor config

I'm just not aware of it. Does it duplicate the GitHub's integration?

[RyanZim]

AppVeyor is a working integration, however, it only does Windows, so we're using GitHub Actions for Linux testing (we used to use TravisCI here, but I just migrated over to GH Actions)

[jimmywarting]

I don't think you need typescript just so you can have typing and esm
just created this a few days ago: https://jimmywarting.github.io/you-might-not-need-typescript/ 😜

I do however approve the move to do esm-only and a major breaking change 🙃