Convert to ESM

[groenroos]

To keep up with The Way Forward, and in order to merge #221, the entire codebase should be converted to ESM.

This still doesn't work properly yet. Importing the render and DB drivers dynamically is broken, as importing is async, but constructors cannot be. It also doesn't seem to be catching exceptions properly for some reason.

It's hard to say yet why, but it seems like some functioning(?) tests are also now returning different values than expected, though nothing else has changed.

I'm also still skeptical if there's actually any benefit to doing this. The app isn't any faster or otherwise better, and ESM doesn't make the code easier to understand or write. In fact, I'd argue that some of the funky patterns make the code actively worse (i.e. (await import('../_utils/app.js')).default(); vs just require('../_utils/app.js')()).

Closes #210

[groenroos]

Converted the entire app, and fixed all the tests - however, the Response tests are still failing with no assertions, likely because the Templating driver dependency hasn't loaded in time for the Response class to use it. Not clear if there's a solution that doesn't require some special syntax to replace every new Response() throughout the app.

[groenroos]

I've managed to cover the async dependency problem with a sort-of kludgey importDriver method in Templating and Storage, and ensuring it is called for each driver-dependent method.

Now it seems that the build behaviour is different for different versions of Node. Node 14 is the most angery, with not even the npm install step completing properly. 12 and 16 complete installation, but have seemingly three tests that time out.

[codecov]

Codecov Report

Merging #222 (d9d9c28) into master (eb36578) will increase coverage by 2.12%.
The diff coverage is 97.48%.

❗ Current head d9d9c28 differs from pull request most recent head e656b9f. Consider uploading reports for the commit e656b9f to get more accurate results

@@            Coverage Diff             @@
##           master     #222      +/-   ##
==========================================
+ Coverage   94.54%   96.67%   +2.12%     
==========================================
  Files          35       35              
  Lines        1247     4449    +3202     
==========================================
+ Hits         1179     4301    +3122     
- Misses         68      148      +80     

Impacted Files	Coverage Δ
lib/Storage.js	85.27% <85.18%> (+2.68%)	⬆️
core/loadServer.js	94.59% <92.85%> (+1.04%)	⬆️
core/initRoute.js	100.00% <100.00%> (ø)
core/loadConfig.js	100.00% <100.00%> (ø)
core/loadController.js	100.00% <100.00%> (ø)
core/loadCustomTags.js	100.00% <100.00%> (ø)
core/loadHooks.js	94.66% <100.00%> (+1.80%)	⬆️
core/loadModel.js	100.00% <100.00%> (ø)
core/loadModules.js	100.00% <100.00%> (ø)
core/loadPermissions.js	100.00% <100.00%> (ø)
... and 60 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb36578...e656b9f. Read the comment docs.

[groenroos]

ESM conversion seems to now be complete, with all tests and linter finally happy. Also tested the app manually, and all major features seem to work fine (and some runtime errors are not caught by current tests, I noticed).

---

There's still some doubt in my mind if this is actually even beneficial.

Pros

• It's ESM now, and so merging Dependabot PRs for Sindre Sorhus' packages should become a lot easier.
• Convert to TypeScript #44 will now be easier to do sometime in the far future

Cons

• Making the dynamic driver imports for Templating and Storage work required polluting the codebase throughout with a call to a "make this class actually work" method each time the class is used - I dislike these types of gotchas
• The new syntax in many places is a lot dirtier and boilerplatey (i.e. CJS' __dirname vs. ESM's path.dirname(fileURLToPath(import.meta.url)) + 2 extra imports; or the example in the PR description) - IMO this makes the intent of what these lines of code are intended to achieve a lot harder to read and understand at a glance
• Had to swap from nyc to c8 for code coverage, which seems a lot slower on local, and carries some risk, as setting up the codecov workflow was a bit hit-and-miss originally - also, the codecov block above is all dark green now for some reason?
• Some xo/eslint rules don't play nice; e.g. the dynamic imports being used as constructors falling foul of class name capitalisation rules; or demanding that yargs/helpers have a file extension, yet adding one breaks the import. For the first time, I had to add per-line exceptions to linting rules.

---

As a side effect of working on this PR, two clear benefits included here are;

1. Fixing the long-broken hanging loadRest test suite
2. Splitting out the CI workflow to test every version of Node separately

However, these benefits aren't dependent on ESM in any way.

[groenroos]

On balance, I think while the conversion to ESM has required some inelegant solutions and upheavals, and carries some (possibly unknown) risk, the requirement for ESM to merge new versions of dependencies will only become more common. And I'd rather import this risk now than under the gun. Hopefully the inelegance can be smoothed over over time.