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

globalPreload docs have non-working code #50279

Open
bmacnaughton opened this issue Oct 19, 2023 · 6 comments
Open

globalPreload docs have non-working code #50279

bmacnaughton opened this issue Oct 19, 2023 · 6 comments
Labels
doc Issues and PRs related to the documentations.

Comments

@bmacnaughton
Copy link
Contributor

bmacnaughton commented Oct 19, 2023

Affected URL(s)

https://nodejs.org/docs/latest-v16.x/api/esm.html#globalpreload
https://nodejs.org/docs/latest-v18.x/api/esm.html#globalpreload

Description of the problem

The example code from the docs doesn't work.

port.onmessage doesn't work. the code needs to be port.on('message', ...). Additionally, there is just data, not evt.data.

I know that globalPreload is gone in 20, so feel free to close this. I am working with making loaders work across multiple versions of node and this was a problem that I ran into.

/**
 * This example has the application context send a message to the loader
 * and sends the message back to the application context
 */
export function globalPreload({ port }) {
  port.onmessage = (evt) => {
    port.postMessage(evt.data);
  };
  return `\
    port.postMessage('console.log("I went to the Loader and back");');
    port.onmessage = (evt) => {
      eval(evt.data);
    };
  `;
}
@bmacnaughton bmacnaughton added the doc Issues and PRs related to the documentations. label Oct 19, 2023
@marco-ippolito
Copy link
Member

Thanks for reporting, would you be interested in opening a PR?

@bmacnaughton
Copy link
Contributor Author

will do

@bmacnaughton
Copy link
Contributor Author

question: should i submit separate PRs for each version of the docs, a single PR based on a single version of the doc, etc? i'm not sure how to proceed with multiple targets. if the first, do i just use, say, the latest version of 16 to update the v16 docs, then another using the latest versions of 18, etc.

@marco-ippolito
Copy link
Member

nope, just a pull request to main with the fix

@targos
Copy link
Member

targos commented Oct 19, 2023

You should submit one PR only, against the highest affected version.
If v21 is affected: PR against main
Otherwise: PR against vN.x-staging, where N is the highest supported and affected version (20 or 18).

@richardlau
Copy link
Member

Also note that Node.js 16 is End-of-Life so will not have any new releases.

aduh95 pushed a commit to bmacnaughton/node that referenced this issue Oct 20, 2023
targos pushed a commit that referenced this issue Oct 28, 2023
Fixes: #50279
PR-URL: #50300
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Fixes: nodejs/node#50279
PR-URL: nodejs/node#50300
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Fixes: nodejs/node#50279
PR-URL: nodejs/node#50300
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

4 participants