Babel removal discussion

[chriszs]

Discussion

What do you want to talk about?

It's been suggested by @traceypooh that the dark days of Babel are behind us and the light of ESM imports in Node is upon us. Let's explore this!

One quick way to enable ESM imports is to add "type": "module" to package.json. This was added behind --experimental-modules flag in Node 12, but not behind a flag as early as 13. (See #50 discussion about what version to standardize on.)

When you add that and run node src/index.js, you get ERR_INVALID_MODULE_SPECIFIER errors caused by differences in how paths are interpreted.

It's very literal. It needs .js at the end of file names, for instance. It wants index.js instead of just pointing at a directory, it seems. It choked on a multi-line import, then when I fixed it, it gave me this error:

import { createLogger, format, transports } from 'winston' ^^^^^^ SyntaxError: The requested module 'winston' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export. For example: import pkg from 'winston'; const { createLogger, format, transports } = pkg;

These seem fixable, though annoying, but less so are module_alias paths prefixed with %src, which it doesn't like.

This seems somewhat harder to resolve. module-alias theoretically supports imports, and does so under Babel, but the way it does so is to hijack every subsequent require. That seems to mean I need to require it? (E.g. require('module-alias/register')). (Correction: it's the /register part it doesn't seem to like. import register from 'module-alias' might work?) And it seems ineffective at rewriting imports if they don't go through Babel.

One pitfall of the require hijack approach (internally it uses a require hijacker called pirates, amusingly, according to my stack traces) is acknowledged in its README:

WARNING: This module should not be used in other npm modules since it modifies the default require behavior! It is designed to be used for development of final projects i.e. web-sites, applications etc.

This could be a barrier to adaptive reuse, even if the core TVKitchen module is primarily for running directly.

There are alternative approaches. One detailed here uses link-module-alias, which creates symlinks as a post-install step run after every npm install or yarn. That post also mentions yarn workspaces as an alternative, one already being used in this project elsewhere.

One benefit is those approaches might need fewer shims, e.g. for ESLint and Jest. But there may be other tradeoffs.

Relevant Resources / Research

• https://arunmichaeldsouza.com/blog/aliasing-module-paths-in-node-js

[reefdog]

Oh man, that module-alias behavior and warning were… not something I had noticed/realized. 😬 I'm very glad you surfaced that.

I laid out my preferences/criteria for a good absolute import solution over in Alerts. We landed on module-alias at the time but it sounds like link-module-alias may be a preferable replacement. Do you think it'll violate any of the criteria on my list there? (I didn't mention, but am fine with, this being part of a post-install/prepare task.)

[slifty]

I love that this was explored, but for now would like to propose we stick with babel for the project. For one thing, it is a dependency that most node devs are familiar enough with that I don't think it is a blocker. I also think that even if node catches up to the standards babel allows, babel will still allow us more control and flexibility to use the most cutting edge options as we feel appropriate.

Most importantly, it would be non-trivial to do and I expect us to have higher priority issues for the forseeable future.

Gien all that I'm going to close this for now (but happy to re-open if anybody feels strongly that it should stay open, and also will gladly revisit this over time!)

Also a note that I would also move it to the meta repo but the exploration done was directly linked to this particular repository.

[chriszs]

I do think we want to consider whether a different approach for aliases makes sense, but perhaps that's a separate issue.

[slifty]

Finally going to spin up an issue for the module aliases stuff you flagged!

[traceypooh]

re: "It's very literal. It needs .js at the end of file names, for instance" and some related thoughts...

kind of annoying - but what's great and worth it about that is the import code is all 100% web browser compatible, with relative import calls

FTR, I re-read this, in detail, at least 3 times over a year, to really get all the ES Modules awesomeness "locked in" to the pinky squishy cranial stuff.

https://hacks.mozilla.org/2018/03/es-modules-a-cartoon-deep-dive/

[chriszs]

It makes total sense when you consider it needs to resolve URLs. I started playing with module resolution logic in the early part of this year and haven't finished because there are so many special cases.

[traceypooh]

There's some other non-obvious minor things to learn / sort out for things like spies and/or sinon in testing, etc.

I spent a couple days trying to figure out how to mock ES Module methods. Ultimately, I came to the totally satisfactory:

https://github.com/traceypooh/js-es-modules-coverage-testing#mocking-stubs-and-spies

I've actually kind of been moving away from JS classes since ES Modules are so great w/ internal constants and methods and only exporting what you like, too.

Over 25,000 happy JS lines, all ES Modules and counting... :)