-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: unflag Module.register
and allow nested loader import()
#48559
esm: unflag Module.register
and allow nested loader import()
#48559
Conversation
Review requested:
|
26b9ab4
to
0bb4399
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned in a code comment, I think this needs to wait until after #48439 lands to know what needs to be done after the refactoring that’s part of that PR.
I read #48515 and I don’t see a consensus there that the current behavior is a problem, and no discussion around what the best potential solution would be. It’s fine to open a PR as a POC to present a potential solution, but I think you need to first make the case for why we need to change the current Loaders API behavior, and then if we agree on that we can look into whether this PR is the best solution.
Also are there no docs updates as part of this? There are no user-visible changes as a result of this PR?
At the very least there should be some new tests, that fail on current main
and pass in this branch, so that the change made by this PR is clear.
Thanks so much for all the feedback @GeoffreyBooth!
Fine with me 😄 looks like the state of that is a bit in flux as it stands? I'm happy to rebase my work on top of it when it's stable enough though.
Yeah this is purely a proposal at this point. Would you prefer I make the case here or in the issue? I thought the issue description highlighted the two primary reasons:
Is there a recommended approach here to continuing the discussion?
There aren't really any user-visible changes. It is (humbly in my opinion) in line with what I see as the spirit of #43772 which states:
This is true ONLY if all the imports of those loaders are static – if one of those loaders invokes
This is my very first time contributing to |
Thanks a lot for sending this PR, Node.js strives from contributions like yours, you did the right thing opening this PR! Welcome and let's make
Could you clarify this point? Your comment makes it sound like
With your PR, we could align static and dynamic behavior:
Am I getting this right?
Aren't you opening this PR because you are a user of the hook API and you would like to observe these changes? A helpful test would be one that's failing today on |
0bb4399
to
d1c068c
Compare
Hey @aduh95!
Thanks for the positive feedback! 😄
That is the idea – In the following example: node \
--loader a \
--loader b Previously import 'magic-thing-that-depends-on-a'; BUT NOT: export const load = async (...) => {
// This fails today
await import('magic-thing-that-depends-on-a');
return ...;
};
export const resolve = async (...) => {
// This fails today
await import('magic-thing-that-depends-on-a');
return ...;
}; This PR changes it so that BOTH cases now work.
Done! Thanks for the suggestion 🎉 |
58d1bd1
to
30cb005
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation and the added test. I agree this is something that should be fixed. I want to keep the block on just to ensure that this doesn't land before #48439, because that one is a cleanup PR to let the new register
API get released and it's important to get that out as soon as we can; and the author of register
is waiting on that and it doesn't feel fair to make the wait even longer because another PR slipped in ahead. If you have time to help with #48439, it covers a lot of the same ground as your PR so you might be able to fix the last few broken tests.
30cb005
to
409484a
Compare
DefaultModuleLoader
Awesome! 😄
I did my best. I apologize profusely for clobbering @JakobJingleheimer's work here 😞 Like you mentioned both PRs kind of overlap a little, and I because I don't understand enough of what's going on in that PR, (first time contributor to I took the test case from @JakobJingleheimer's PR and it passes, as well as the standard Thanks again @aduh95 and @GeoffreyBooth for your words of encouragement and feedback throughout all this 🎉 |
409484a
to
76f775e
Compare
lib/internal/modules/esm/loader.js
Outdated
return this.#delegate.register(specifier, parentUrl); | ||
} | ||
// TODO: Consider flagging this. | ||
const flag = true; // getOptionValue('--experimental-loader-register'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want a flag like --experimental-loader-register
@aduh95 or @GeoffreyBooth and show me how I can implement it then I'm happy to add this here. Otherwise this unflags things in the same manner as https://github.com/nodejs/node/pull/48439/files#r1227723703.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to make a decision on this @aduh95 @JakobJingleheimer. I’m considering two options:
- Introduce a new flag like
--experimental-module-customization
that needs to be passed in order to enable the Loaders API at all, whether viaregister
or--experimental-loader
/--loader
. - Decide that we want to unflag the Loaders API, while keeping the printed warning and the experimental status in the docs. We should probably add a special printed warning for
globalPreload
to say explicitly that it’s disappearing soon.
The advantage of the new flag is to undo the mistake we made of shipping --loader
by itself, and really emphasizes to users that this stuff is still experimental. But we’re so close to stability that I’m not sure we want to send that message; we could go the other way and inch closer to stability by dropping the need for a flag, like going to a closer-to-stable experimental status. I’m more inclined to do the latter, as it’s less user-hostile and arguably not much of a change from the status quo where --loader
exists. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my (an end-user's) perspective the act of invoking register
is kind of? acknowledging the opt-in (you can't accidentally turn the behaviour on unless you depend on some third-party code that does I guess) and I think your latter suggestion is pretty sensible as it aligns with what I was consider default intuition. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, GitHub collapsed this and I missed it.
I think ideally we do neither, and instead enable --experimental-loader
to be either (effectively) a boolean or a list. If that's not possible, then land but don't release 😢
So does this PR now both fix the Would it be feasible to split this into two PRs, one for the |
Yes, to my knowledge! 🎉 The programmatic usage of
This might be possible… it would require a bit of finessing… I'll have to think on this. Let me know how critical this is as it might require some effort 😄 |
@GeoffreyBooth @aduh95 how would you like me to proceed with this? Should I keep it as-is and wait for feedback? Should I continue iterating on a few possible cleanup ideas I have? |
I'm traveling until July 12; @JakobJingleheimer is on holiday until July 8. I'll review it when I'm back, and I think this should definitely get Jacob's review too before it lands. Sorry you happened to submit this during a holiday period 😀 If you have improvements that don't increase the scope of the PR any further, sure, do them on this branch; but anything that can be logically separated as a follow-up PR would be better saved for that, so things are easier to review and bisect in case of issues. You can branch off of this branch and even open the follow-up with a note that it's based on this, and ideally include the link to compare the diff of the two branches so that someone can review the subsequent improvements before waiting for this to land. Thanks for your work and thanks for asking! |
7170a68
to
cb657fb
Compare
Ack sorry I didn't realize you were off, enjoy your travels!
Thanks for clarifying! 😄 I think I've made the rest of the improvements I want to… as well as addressed the bulk of the cleanup points I made for myself. I'll wait til you're both back before following up further. |
Notable changes: esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185
Notable changes: doc: * add new TSC members (Michael Dawson) #48841 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185
Notable changes: doc: * add new TSC members (Michael Dawson) #48841 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185
Notable changes: doc: * add new TSC members (Michael Dawson) #48841 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185
Notable changes: doc: * add new TSC members (Michael Dawson) #48841 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185
Notable changes: doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 PR-URL: #49185
deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185
Based on the note from @GeoffreyBooth on #46826 (comment) I'm going ahead and adding a backport-requested label to this PR too. Ideally you can organize it so that it can be a single backport PR containing all the required commits 😊 |
Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) nodejs#48660 doc: * add new TSC members (Michael Dawson) nodejs#48841 * add rluvaton to collaborators (Raz Luvaton) nodejs#49215 esm: * unflag import.meta.resolve (Guy Bedford) nodejs#49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) nodejs#48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) nodejs#48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) nodejs#48765 module: * implement `register` utility (João Lenon) nodejs#46826 * make CJS load from ESM loader (Antoine du Hamel) nodejs#47999 src: * add built-in `.env` file support (Yagiz Nizipli) nodejs#48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) nodejs#48660 and nodejs#45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) nodejs#48975 PR-URL: nodejs#49185
Major functional changes: - Allow `import()` to work within loaders that require other loaders, - Unflag the use of `Module.register`. A new interface `Customizations` has been created in order to unify `ModuleLoader` (previously `DefaultModuleLoader`), `Hooks` and `CustomizedModuleLoader` all of which now implement it: ```ts interface LoadResult { format: ModuleFormat; source: ModuleSource; } interface ResolveResult { format: string; url: URL['href']; } interface Customizations { allowImportMetaResolve: boolean; load(url: string, context: object): Promise<LoadResult> resolve( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ): Promise<ResolveResult> resolveSync( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ) ResolveResult; register(specifier: string, parentUrl: string): any; forceLoadHooks(): void; importMetaInitialize(meta, context, loader): void; } ``` The `ModuleLoader` class now has `setCustomizations` which takes an object of this shape and delegates its responsibilities to this object if present. Note that two properties `allowImportMetaResolve` and `resolveSync` exist now as a mechanism for `import.meta.resolve` – since `Hooks` does not implement `resolveSync` other loaders cannot use `import.meta.resolve`; `allowImportMetaResolve` is a way of checking for that case instead of invoking `resolveSync` and erroring. Fixes nodejs#48515 Closes nodejs#48439 PR-URL: nodejs#48559 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Major functional changes: - Allow `import()` to work within loaders that require other loaders, - Unflag the use of `Module.register`. A new interface `Customizations` has been created in order to unify `ModuleLoader` (previously `DefaultModuleLoader`), `Hooks` and `CustomizedModuleLoader` all of which now implement it: ```ts interface LoadResult { format: ModuleFormat; source: ModuleSource; } interface ResolveResult { format: string; url: URL['href']; } interface Customizations { allowImportMetaResolve: boolean; load(url: string, context: object): Promise<LoadResult> resolve( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ): Promise<ResolveResult> resolveSync( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ) ResolveResult; register(specifier: string, parentUrl: string): any; forceLoadHooks(): void; importMetaInitialize(meta, context, loader): void; } ``` The `ModuleLoader` class now has `setCustomizations` which takes an object of this shape and delegates its responsibilities to this object if present. Note that two properties `allowImportMetaResolve` and `resolveSync` exist now as a mechanism for `import.meta.resolve` – since `Hooks` does not implement `resolveSync` other loaders cannot use `import.meta.resolve`; `allowImportMetaResolve` is a way of checking for that case instead of invoking `resolveSync` and erroring. Fixes #48515 Closes #48439 PR-URL: #48559 Backport-PR-URL: #50669 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Major functional changes: - Allow `import()` to work within loaders that require other loaders, - Unflag the use of `Module.register`. A new interface `Customizations` has been created in order to unify `ModuleLoader` (previously `DefaultModuleLoader`), `Hooks` and `CustomizedModuleLoader` all of which now implement it: ```ts interface LoadResult { format: ModuleFormat; source: ModuleSource; } interface ResolveResult { format: string; url: URL['href']; } interface Customizations { allowImportMetaResolve: boolean; load(url: string, context: object): Promise<LoadResult> resolve( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ): Promise<ResolveResult> resolveSync( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ) ResolveResult; register(specifier: string, parentUrl: string): any; forceLoadHooks(): void; importMetaInitialize(meta, context, loader): void; } ``` The `ModuleLoader` class now has `setCustomizations` which takes an object of this shape and delegates its responsibilities to this object if present. Note that two properties `allowImportMetaResolve` and `resolveSync` exist now as a mechanism for `import.meta.resolve` – since `Hooks` does not implement `resolveSync` other loaders cannot use `import.meta.resolve`; `allowImportMetaResolve` is a way of checking for that case instead of invoking `resolveSync` and erroring. Fixes nodejs/node#48515 Closes nodejs/node#48439 PR-URL: nodejs/node#48559 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Major functional changes: - Allow `import()` to work within loaders that require other loaders, - Unflag the use of `Module.register`. A new interface `Customizations` has been created in order to unify `ModuleLoader` (previously `DefaultModuleLoader`), `Hooks` and `CustomizedModuleLoader` all of which now implement it: ```ts interface LoadResult { format: ModuleFormat; source: ModuleSource; } interface ResolveResult { format: string; url: URL['href']; } interface Customizations { allowImportMetaResolve: boolean; load(url: string, context: object): Promise<LoadResult> resolve( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ): Promise<ResolveResult> resolveSync( originalSpecifier: string, parentURL: string, importAssertions: Record<string, string> ) ResolveResult; register(specifier: string, parentUrl: string): any; forceLoadHooks(): void; importMetaInitialize(meta, context, loader): void; } ``` The `ModuleLoader` class now has `setCustomizations` which takes an object of this shape and delegates its responsibilities to this object if present. Note that two properties `allowImportMetaResolve` and `resolveSync` exist now as a mechanism for `import.meta.resolve` – since `Hooks` does not implement `resolveSync` other loaders cannot use `import.meta.resolve`; `allowImportMetaResolve` is a way of checking for that case instead of invoking `resolveSync` and erroring. Fixes nodejs/node#48515 Closes nodejs/node#48439 PR-URL: nodejs/node#48559 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Overview
Major functional changes:
import()
to work within loaders that require other loaders,Module.register
.A new interface
Customizations
has been created in order to unifyModuleLoader
(previouslyDefaultModuleLoader
),Hooks
andCustomizedModuleLoader
all of which now implement it:The
ModuleLoader
class now hassetCustomizations
which takes an object of this shape and delegates its responsibilities to this object if present.Note that two properties
allowImportMetaResolve
andresolveSync
exist now as a mechanism forimport.meta.resolve
– sinceHooks
does not implementresolveSync
other loaders cannot useimport.meta.resolve
;allowImportMetaResolve
is a way of checking for that case instead of invokingresolveSync
and erroring.Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.