Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESM loader #5447

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

ESM loader #5447

wants to merge 4 commits into from

Conversation

edemaine
Copy link
Contributor

@edemaine edemaine commented May 1, 2023

Following up on my offer from #5018 (comment), here is an ESM loader for CoffeeScript that (once published) enables the following:

node --loader coffeescript/esm filename.coffee

For example, I've tested with the main.coffee/scream.coffee examples from https://nodejs.org/api/esm.html#esm_transpiler_loader (on which this code is based). You can test too via npm link.

Potential issues for discussion

Currently the loader assumes that, if you're using an ESM loader, the desire is to load code in ESM mode. I think this is often what people would want, but it's possible that the user is writing ESM code that imports a CJS module written in CoffeeScript. In that case, the user should ideally use CoffeeScript's register. But if the user has a mix of CoffeeScript ESM and CJS code, the ESM loader will probably break import of CJS code.

By contrast, https://nodejs.org/api/esm.html#esm_transpiler_loader uses package.json's type field. But given the lack of an .mjs-style extension, this is not a very flexible system. I also worry about the performance hit of traversing up the filesystem (which is often very expensive on Windows).

Another alternative would be to look for import/export statements in the output, and use that to turn on ESM mode. Top-level await is maybe trickier to detect though. (Maybe via the AST.)

Another thing I've not yet tested is combining with another loader such as babel-register-esm.

If any of the above are priorities before an initial release, I'd be happy to work on them given a desired direction. But hopefully this is a useful first step toward good ESM support.

@GeoffreyBooth
Copy link
Collaborator

I think we might want to wait a bit to see what happens to nodejs/node#47880. Once there’s a clear way to handle both ESM and CJS code through a loader, it should be much easier to handle realistic use cases. For example I don’t know if it’s plausible to assume an app has no CommonJS in its module graph, when you take node_modules dependencies into account. I don’t think this needs to be perfect before it lands, but handling CommonJS along with ESM seems like something to figure out before introducing this to users since the DX might change based on how things go (like we may need to do something like --import coffeescript/register --loader coffeescript/loader or something).

As for the type field, that field exists because of CoffeeScript’s need to have a way other than file extension to know whether a file should be treated as CommonJS or ESM. The expectation was that that’s what we would use. You shouldn’t need to walk the filesystem for every file, just for every folder; once the nearest parent package.json is known for a particular folder, that can’t change for all the other files in that folder, so that should be something you can cache and reuse without filesystem calls. I think it’s also reasonable to write the loader to stop at the node_modules boundary, handing off to Node anything that’s under there. (This would mean no packages written in CoffeeScript and run as CoffeeScript; packages would need to be transpiled first and the .js output saved in the folder under node_modules. But I think the performance benefit should be worth it.)

Anyway I’m not trying to add blockers; if you’ve thought through the various CommonJS use cases and think they’re all handled, please just explain them and I’m happy to trust you.

@GeoffreyBooth
Copy link
Collaborator

GeoffreyBooth commented Sep 8, 2023

Node 20.6.0 had the ESM hooks refactor I was waiting for. The module customization hooks in Node are now in their likely final shape, and we can refactor this PR to provide a way to transpile and run ESM CoffeeScript.

First off, don’t use globalPreload. It’s going away soon. The basic flow now that module customization hooks run off-thread is this:

node --import coffeescript/register app.coffee

Inside register.coffee add { register } = require 'node:module' and call register to have Node load the file that defines the hooks. Leave the rest of the existing code that hooks up CoffeeScript to transpile modules referenced via require. This way applications with a mix of CommonJS and ESM syntax can work.

That other file, hooks.coffee or whatever we call it, should define at least load along the lines of https://github.com/nodejs/node/blob/main/doc/api/module.md#transpilation. You might also want to use initialize and establish a communications port with the application thread, to avoid transpiling the same module twice (for example, if it gets both imported and required); see https://github.com/nodejs/node/blob/main/doc/api/module.md#initialize.

Getting the coffee command to work could follow in a subsequent PR. There’s still no support in vm for running ESM scripts, but perhaps there’s a different solution where node:module register is used to register the hooks before importing the entry point, and then the same flow as node --import handles the full app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants