GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — esm: add `initialize` hook, integrate with `register` by izaakschroeder · Pull Request #48842 · nodejs/node

Conversation

@izaakschroeder

@izaakschroeder izaakschroeder commented Jul 19, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Follows @giltayar's proposed API:

register can pass any data it wants to the loader, which will be passed to the exported initialize function of the loader. Additionally, if the user of register wants to communicate with the loader, it can just create a MessageChannel and pass the port to the loader as data.

The register API is now:

interface Options {
  parentUrl?: string;
  data?: any;
  transferList?: any[];
}

function register(loader: string, parentUrl?: string): any;
function register(loader: string, options?: Options): any;

This API is backwards compatible with the old one (new arguments are optional and at the end) and allows for passing data into the new initialize hook. If this hook returns data it is passed back to register:

function initialize(data: any): Promise<any>;

NOTE: Currently there is no mechanism for a loader to exchange ownership of something back to the caller.

Refs: nodejs/loaders#147


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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

shrujalshah28 reacted with thumbs up emoji JakobJingleheimer and GeoffreyBooth reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 2 reactions
@nodejs-github-bot

nodejs-github-bot commented Jul 19, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/modules
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Jul 19, 2023
@izaakschroeder
izaakschroeder marked this pull request as ready for review July 19, 2023 22:03
@izaakschroeder

izaakschroeder commented Jul 19, 2023

Copy link
Copy Markdown
Contributor Author

/cc @@targos @giltayar

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anonrig
anonrig reviewed Jul 19, 2023
View reviewed changes
Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

ljharb
ljharb reviewed Jul 19, 2023
View reviewed changes
Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Jul 19, 2023

Copy link
Copy Markdown
Member

This API is backwards compatible with the old one (new arguments are optional and at the end)

register hasn’t been released yet, so we don’t need to preserve any backward compatibility. That said, the proposed API seems fine. (Do we need parentURL? It seems like it’s already optional and set to process.cwd if undefined, so maybe we can do without it and users can pass as the first argument a path to the loader relative to CWD or an absolute URL to the loader.)

The docs need updating as part of this PR. That would help in reviews, as we can see the intended usage from the example in the new docs. Since register should work in CommonJS too, it would be good to have both CommonJS and ESM versions of the examples.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth GeoffreyBooth added the loaders Issues and PRs related to ES module loaders. label Jul 20, 2023
@GeoffreyBooth GeoffreyBooth mentioned this pull request Jul 20, 2023
fix: Add globalPreload to ts-node/esm for node 20 TypeStrong/ts-node#2009
Open
aduh95
aduh95 reviewed Jul 20, 2023
View reviewed changes
Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 reviewed Jul 20, 2023
View reviewed changes
Comment thread test/fixtures/es-module-loaders/hooks-initialize.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 reviewed Jul 20, 2023
View reviewed changes
Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 reviewed Jul 20, 2023
View reviewed changes
Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 reviewed Jul 20, 2023
View reviewed changes
Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@izaakschroeder

izaakschroeder commented Jul 21, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for all the feedback! Work is a bit busy, but I will get to this later tonight (Friday) 😄

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@izaakschroeder

izaakschroeder commented Jul 21, 2023

Copy link
Copy Markdown
Contributor Author

The docs need updating as part of this PR. That would help in reviews, as we can see the intended usage from the example in the new docs. Since register should work in CommonJS too, it would be good to have both CommonJS and ESM versions of the examples.

@GeoffreyBooth how/where do I update the docs?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Jul 21, 2023

Copy link
Copy Markdown
Member

how/where do I update the docs?

Edit the module.md file in the repo: https://github.com/nodejs/node/blob/main/doc/api/module.md#moduleregister

You’ll find loaders documented in esm.md: https://github.com/nodejs/node/blob/main/doc/api/esm.md#loaders

izaakschroeder reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@izaakschroeder

izaakschroeder commented Jul 22, 2023

Copy link
Copy Markdown
Contributor Author

@GeoffreyBooth while I do up the docs, is the CJS test here sufficient or do you want to see something more?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Jul 22, 2023

Copy link
Copy Markdown
Member

is the CJS test here sufficient or do you want to see something more?

We should add a test that specifically uses --require, not just --eval, as the former is a different code path that runs at a different point in startup. Basically, I expect some users to run commands like node --require ts-node/register app.js and that should work as expected. Bonus points for setting up a require.extensions hook that does something and it's coordinated with the load hook. That simulates the transpilation use case.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@izaakschroeder

izaakschroeder commented Jul 22, 2023

Copy link
Copy Markdown
Contributor Author

@GeoffreyBooth I added two more tests but one of them is failing. I added {skip: ...} for now but I will have to do further investigation. Let me know if you have any ideas on the source of this failure 😄

JakobJingleheimer reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Jul 22, 2023

Copy link
Copy Markdown
Member

Please don't add skip, we don't want to accidentally land a PR with failing tests. It's okay if CI fails while we're still working.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@izaakschroeder

izaakschroeder commented Jul 22, 2023

Copy link
Copy Markdown
Contributor Author

Oh, sorry, removing skip 😅

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JakobJingleheimer
JakobJingleheimer reviewed Jul 22, 2023
View reviewed changes

@JakobJingleheimer JakobJingleheimer left a comment

Copy link
Copy Markdown
Member

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.

This is looking good!

If you'd like help with anything, I have some time this weekend (and then I'm away starting Wednesday for ~2 weeks)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/hooks.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/es-module/test-esm-loader-hooks.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@JakobJingleheimer

JakobJingleheimer commented Jul 22, 2023

Copy link
Copy Markdown
Member

Do we need parentURL?

@GeoffreyBooth yes: this addresses a specific need; I can't remember what it is, but I'm sure it was discussed. I can try to find it if you'd like.

aduh95 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JakobJingleheimer JakobJingleheimer mentioned this pull request Jul 22, 2023
ESM Loaders breaking changes notification nodejs/loaders#103
Open
@GeoffreyBooth

GeoffreyBooth commented Jul 22, 2023

Copy link
Copy Markdown
Member

@GeoffreyBooth yes: this addresses a specific need; I can’t remember what it is, but I’m sure it was discussed. I can try to find it if you’d like.

Yes, if you can find the reason, I’d appreciate it. I assume it’s because we can’t figure out how to search for specifier relative to the importing file. Assuming that’s the reason, I still think I’d like to remove parentUrl from the signature. It defaults to process.cwd, so I think we could eliminate it and anyone who would have been passing it (because specifier is relative to some base other than CWD) can instead call register(new URL(specifier, parentURL).href, data, transferList).

And if specifier cannot be found, the error message should say something about Node searching for it relative to the current working directory, rather than relative to the importing file. I think we should probably add this extra error text regardless of whether or not parentUrl stays or goes, as I think this “relative to CWD” behavior is atypical for Node and potentially surprising.

How do others feel about this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos

targos commented Jul 22, 2023

Copy link
Copy Markdown
Member

I would prefer the signature to be loader, options?.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aduh95

aduh95 commented Jul 22, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

can instead call register(new URL(specifier, parentURL).href, data, transferList).

You would not get the expected result if you try to register a node_module package.

because specifier is relative to some base other than CWD

Note that's almost certainly the case, the only case I can think of where resolving relative to the CWD makes sense is to parse a relative path passed as CLI argument, I expect this will not be the case for the vast majority of register calls.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Jul 22, 2023

Copy link
Copy Markdown
Member

You would not get the expected result if you try to register a node_module package.

For a package you would just pass the specifier, e.g. register('ts-node'). My example was only for custom loaders, like register(new URL('./my-custom-loader.js', import.meta.url).href).

the only case I can think of where resolving relative to the CWD makes sense is to parse a relative path passed as CLI argument

Yes, except that’s how register currently works as far as I know. I assume we all agree that it would be nice if it worked relative to the current file, like how import() works, if anyone can figure out how to make that happen.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

RafaelGSS pushed a commit that referenced this pull request Aug 17, 2023
@izaakschroeder @RafaelGSS
esm: add initialize hook, integrate with register
386e826
Follows @giltayar's proposed API:

> `register` can pass any data it wants to the loader, which will be
passed to the exported `initialize` function of the loader.
Additionally, if the user of `register` wants to communicate with the
loader, it can just create a `MessageChannel` and pass the port to the
loader as data.

The `register` API is now:

```ts
interface Options {
  parentUrl?: string;
  data?: any;
  transferList?: any[];
}

function register(loader: string, parentUrl?: string): any;
function register(loader: string, options?: Options): any;
```

This API is backwards compatible with the old one (new arguments are
optional and at the end) and allows for passing data into the new
`initialize` hook. If this hook returns data it is passed back to
`register`:

```ts
function initialize(data: any): Promise<any>;
```

**NOTE**: Currently there is no mechanism for a loader to exchange
ownership of something back to the caller.

Refs: nodejs/loaders#147
PR-URL: #48842
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
UlisesGascon added a commit that referenced this pull request Aug 17, 2023
@UlisesGascon
2023-08-16, Version 20.6.0 (Current)
c9c813a
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
UlisesGascon added a commit that referenced this pull request Aug 17, 2023
@UlisesGascon
2023-08-21, Version 20.6.0 (Current)
dfe3820
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
UlisesGascon added a commit that referenced this pull request Aug 17, 2023
@UlisesGascon
2023-08-21, Version 20.6.0 (Current)
44d51da
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
UlisesGascon added a commit that referenced this pull request Aug 18, 2023
@UlisesGascon
2023-08-21, Version 20.6.0 (Current)
6673b86
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
UlisesGascon added a commit that referenced this pull request Aug 22, 2023
@UlisesGascon
2023-08-23, Version 20.6.0 (Current)
4148741
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
UlisesGascon added a commit that referenced this pull request Aug 24, 2023
@UlisesGascon
2023-08-28, Version 20.6.0 (Current)
df92c38
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
UlisesGascon added a commit that referenced this pull request Aug 29, 2023
@UlisesGascon
2023-08-30, Version 20.6.0 (Current)
4e4bb9f
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
UlisesGascon added a commit that referenced this pull request Aug 31, 2023
@UlisesGascon
2023-09-04, Version 20.6.0 (Current)
da933e9
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
UlisesGascon added a commit that referenced this pull request Sep 1, 2023
@UlisesGascon
2023-09-04, Version 20.6.0 (Current)
9700344
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
juanarbol pushed a commit that referenced this pull request Sep 4, 2023
@UlisesGascon @juanarbol
2023-09-04, Version 20.6.0 (Current)
1235426
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
@ruyadorno ruyadorno added the backport-requested-v18.x label Sep 7, 2023
@ruyadorno

ruyadorno commented Sep 7, 2023

Copy link
Copy Markdown
Member

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 😊

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aduh95 aduh95 mentioned this pull request Sep 7, 2023
doc: update outdated history info #49530
Merged
nodejs-github-bot pushed a commit that referenced this pull request Sep 7, 2023
@aduh95
doc: update outdated history info
cd49408
PR-URL: #49530
Refs: #48842
Refs: #47999
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@aduh95 @ruyadorno
doc: update outdated history info
d0e0eb4
PR-URL: #49530
Refs: #48842
Refs: #47999
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Nov 11, 2023
@izaakschroeder @targos
esm: add initialize hook, integrate with register
c34870e
Follows @giltayar's proposed API:

> `register` can pass any data it wants to the loader, which will be
passed to the exported `initialize` function of the loader.
Additionally, if the user of `register` wants to communicate with the
loader, it can just create a `MessageChannel` and pass the port to the
loader as data.

The `register` API is now:

```ts
interface Options {
  parentUrl?: string;
  data?: any;
  transferList?: any[];
}

function register(loader: string, parentUrl?: string): any;
function register(loader: string, options?: Options): any;
```

This API is backwards compatible with the old one (new arguments are
optional and at the end) and allows for passing data into the new
`initialize` hook. If this hook returns data it is passed back to
`register`:

```ts
function initialize(data: any): Promise<any>;
```

**NOTE**: Currently there is no mechanism for a loader to exchange
ownership of something back to the caller.

Refs: nodejs/loaders#147
PR-URL: nodejs#48842
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
@merceyz merceyz mentioned this pull request Nov 11, 2023
fix(pnp) esm - support loaders importing named exports from commonjs yarnpkg/berry#5961
Merged
3 tasks done
@targos targos mentioned this pull request Nov 11, 2023
[v18.x] Backport most ESM and customization hook changes #50669
Closed
arcanis pushed a commit to yarnpkg/berry that referenced this pull request Nov 14, 2023
@merceyz
fix(pnp) esm - support loaders importing named exports from commonjs (#…
6e920f9
…5961)

**What's the problem this PR addresses?**

nodejs/node#48842 changed it so that our fs
patch is loaded after the internal translators so ESM importing named
CJS exports in loaders doesn't work.

Fixes #5951

**How did you fix it?**

Updated our ESM loader patching to target the `fs` bindings used
internally.

**Checklist**
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).
- [x] I have set the packages that need to be released for my changes to
be effective.
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
@targos targos mentioned this pull request Nov 28, 2023
v18.19.0 release proposal #50953
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes
@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes
@aduh95 aduh95 aduh95 approved these changes
@ljharb ljharb Awaiting requested review from ljharb
@JakobJingleheimer JakobJingleheimer Awaiting requested review from JakobJingleheimer
@anonrig anonrig Awaiting requested review from anonrig
+1 more reviewer
@targos targos targos left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders.

Projects

None yet

Milestone

ESM Loaders: Stable (CJS parity)

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

11 participants

@izaakschroeder @nodejs-github-bot @GeoffreyBooth @JakobJingleheimer @targos @aduh95 @giltayar @ruyadorno @ljharb @mcollina @anonrig
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.