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 Loaders breaking changes notification #103

Open
JakobJingleheimer opened this issue Jul 24, 2022 · 8 comments
Open

ESM Loaders breaking changes notification #103

JakobJingleheimer opened this issue Jul 24, 2022 · 8 comments
Assignees

Comments

@JakobJingleheimer
Copy link
Contributor

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 JakobJingleheimer self-assigned this Jul 24, 2022
@nodejs nodejs locked and limited conversation to collaborators Jul 24, 2022
@JakobJingleheimer
Copy link
Contributor Author

cc:

@giltayar
Copy link
Contributor

❤️

@arcanis
Copy link
Contributor

arcanis commented Jan 19, 2023

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 nodejs/node#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 nodejs/node#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.

@nodejs nodejs unlocked this conversation Jan 20, 2023
@cspotcode
Copy link
Contributor

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.

@JakobJingleheimer
Copy link
Contributor Author

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.

@bizob2828

This comment was marked as off-topic.

@nodejs nodejs locked and limited conversation to collaborators Jul 27, 2023
@JakobJingleheimer
Copy link
Contributor Author

JakobJingleheimer commented Sep 13, 2023

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.

@GeoffreyBooth
Copy link
Member

FYI nodejs/node#50669 will backport the changes on 20 (register, etc.) to 18.x.

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

No branches or pull requests

6 participants