loaders icon indicating copy to clipboard operation
loaders copied to clipboard

ESM Loaders breaking changes notification

Open JakobJingleheimer opened this issue 2 years ago • 8 comments

Since Node.js has no official way of notifying package authors of incoming breaking changes, and Loaders has had and likely will have a couple more before stability, I'll post incoming breaking changes to this thread before landing them. When a breaking change is about to land, one of us will post a message here with a link to the PR

To avoid spamming authors who just want the change notifications, I'm locking the thread.

JakobJingleheimer avatar Jul 24 '22 09:07 JakobJingleheimer

cc:

  • esmock @bumblehead
  • mocha, testdouble @giltayar
  • ts-node @cspotcode
  • yarn @arcanis

JakobJingleheimer avatar Jul 24 '22 10:07 JakobJingleheimer

❤️

giltayar avatar Jul 25 '22 06:07 giltayar

Until now, loaders didn't affect the resolution / load steps of subsequent loaders. For example, it wasn't possible to have loaders written in TypeScript, because the TypeScript loader wouldn't be used to resolve and load subsequent loaders:

node --loader typescript --loader ./my-loader.ts

We merged PR https://github.com/nodejs/node/pull/43772 that fixes that. While I see it as a bugfix, it may also be seen as a breaking change in that if you have a loader that breaks the resolution, as it'll also prevent subsequent loaders from being properly loaded. For example, the following wouldn't work anymore:

node --loader ./my-loader-that-always-return-"foo.ts" --loader typescript

I'd like to land it in the next 19.x release to collect early feedback and unblock use cases that rely on multiple loaders. Is everyone fine with that? The alternative is to wait for https://github.com/nodejs/node/pull/44710 to land, to try to batch together PRs that have the potential to break something somewhere - but with off-threading being a much larger / riskier change, I'm not convinced batching them is a good idea. I'd personally prefer to incrementally land PRs, rather than all at once.

arcanis avatar Jan 19 '23 17:01 arcanis

Sounds good to me, agreed that iterating is easier for the ecosystem than batching. I also suspect the off-thread changes are gonna need more work till they're merge-able.

cspotcode avatar Jan 20 '23 19:01 cspotcode

Status update for loader authors:

We will be deprecating the globalPreload hook and replacing it with a new initialize hook that facilitates arbitrary data transfer and message channelling with the loader worker. We will introduce the new hook and have a deprecation period for migration before removing old globalPreload. The PR for this appears close to landing.

At the same time as releasing the new hook and deprecating the old, we plan to release module.register() for programmatically registering a loader; this has already landed but was held back from release to avoid potential breaking changes (one of the leading designs was breaking).

We are also in the process of handling CJS loading via ESMLoader when ESM is involved (there are various conditions for this). This means custom loaders will now be able to supply CJS source (prior to this change, source from a loader with format: 'commonjs' was ignored). We intend for this to be a transparent change; however, we are aware of only so many use-cases. Contributions of test-cases are highly encouraged and welcomed so we can ensure they continue to work as authors expect.

JakobJingleheimer avatar Jul 22 '23 15:07 JakobJingleheimer

@JakobJingleheimer I posted a comment here. I think I finally found the answer which is explained in your comment above. I'm struggling to visualize because it appears the initialize ticket hasn't been addressed. I'm really just looking for some resources to help me solve instrumentation ESM packages in a loader within Node 20 due to all the changes.

bizob2828 avatar Jul 27 '23 16:07 bizob2828

Status update for hook authors:

The globalPreload hook has been removed on main, and will not be present in the v21.0.0 release in October. This change may later be back-ported to v20.x (but likely not v18.x).

On a related note, module.register and the initialize hook are available now, and are intended as replacements to globalPreload. We migrated our existing globalPreload test-cases to register + initialize, and all cases continue to work (and the migration was fairly straight-forward—no design changes, mostly deleting now-obsolete code).

Customization Hooks will also become Release Candidate as of node v21.0.0. This means we do not anticipate significant changes. They will remain RC for some time for issues to surface, and after those have been addressed, the API will be marked stable.

Edit: an earlier version of this post said the globalPreload removal would likely be back-ported to v18.x.

JakobJingleheimer avatar Sep 13 '23 08:09 JakobJingleheimer

FYI https://github.com/nodejs/node/pull/50669 will backport the changes on 20 (register, etc.) to 18.x.

GeoffreyBooth avatar Nov 12 '23 17:11 GeoffreyBooth