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

Loader preload code never executes #48778

Closed
bojavou opened this issue Jul 15, 2023 · 1 comment · Fixed by #48779
Closed

Loader preload code never executes #48778

bojavou opened this issue Jul 15, 2023 · 1 comment · Fixed by #48779
Labels
confirmed-bug Issues with confirmed bugs. loaders Issues and PRs related to ES module loaders

Comments

@bojavou
Copy link

bojavou commented Jul 15, 2023

Version

v20.0.0

Platform

Linux silica 5.19.0-45-generic #46-Ubuntu SMP PREEMPT_DYNAMIC Wed Jun 7 09:08:58 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

Loader API

What steps will reproduce the bug?

  • Save this code to loader.mjs.
    export function globalPreload () {
      console.log('PRELOAD')
      return `
    const console = getBuiltin('console')
    console.log('BOOTSTRAP')
    `
    }
  • On any v20 release, run node --loader=./loader.mjs.
  • PRELOAD message is logged. BOOTSTRAP message is never logged.

How often does it reproduce? Is there a required condition?

Consistently on any v20 release.

What is the expected behavior? Why is that the expected behavior?

The string returned by globalPreload() is executed on the main thread. The API docs claim it will happen.

What do you see instead?

The string returned by globalPreload() is never executed.

Additional information

On the latest v19 this code works.

$ nvm use v19
Now using node v19.9.0 (npm v9.6.7)
$ node --loader=./loader.mjs
(node:509065) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
PRELOAD
BOOTSTRAP
Welcome to Node.js v19.9.0.

On the earliest v20 the returned string never executes.

$ nvm use v20.0.0
Now using node v20.0.0 (npm v9.6.4)
$ node --loader=./loader.mjs
Welcome to Node.js v20.0.0.
Type ".help" for more information.
> (node:516773) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
PRELOAD

>

I tried setting a global property in the bootstrap code to ensure it wasn't just a console issue. The property is never set.

export function globalPreload () {
  console.log('PRELOAD')
  return `
globalThis.test = 1
`
}
$ nvm use v19
Now using node v19.9.0 (npm v9.6.7)
$ node --loader=./loader.mjs
(node:523929) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
PRELOAD
Welcome to Node.js v19.9.0.
Type ".help" for more information.
> test
1
$ nvm use v20.0.0
Now using node v20.0.0 (npm v9.6.4)
$ node --loader=./loader.mjs
Welcome to Node.js v20.0.0.
Type ".help" for more information.
> (node:521480) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
PRELOAD

> test
Uncaught ReferenceError: test is not defined

Could have sworn I saw this working before on v20, but it fails consistently now. I tried it on 2 different machines, one with fresh install of Node.js, with the same result everywhere.

Would really like to use the loader thread. It seems like they should be isolated. But I've got to stick with v19 until I can run a bootstrap at startup.

@aduh95
Copy link
Contributor

aduh95 commented Jul 15, 2023

I'm able to reproduce with node --experimental-loader 'data:text/javascript,export function globalPreload(){return"throw null"}' /cc @nodejs/loaders

@aduh95 aduh95 added confirmed-bug Issues with confirmed bugs. loaders Issues and PRs related to ES module loaders labels Jul 15, 2023
aduh95 added a commit that referenced this issue Jul 17, 2023
PR-URL: #48779
Fixes: #48778
Fixes: #48516
Refs: #46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
pluris pushed a commit to pluris/node that referenced this issue Aug 6, 2023
PR-URL: nodejs#48779
Fixes: nodejs#48778
Fixes: nodejs#48516
Refs: nodejs#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
pluris pushed a commit to pluris/node that referenced this issue Aug 7, 2023
PR-URL: nodejs#48779
Fixes: nodejs#48778
Fixes: nodejs#48516
Refs: nodejs#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48779
Fixes: nodejs#48778
Fixes: nodejs#48516
Refs: nodejs#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48779
Fixes: nodejs#48778
Fixes: nodejs#48516
Refs: nodejs#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48779
Fixes: nodejs#48778
Fixes: nodejs#48516
Refs: nodejs#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
RafaelGSS pushed a commit that referenced this issue Aug 15, 2023
PR-URL: #48779
Fixes: #48778
Fixes: #48516
Refs: #46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
targos pushed a commit to targos/node that referenced this issue Nov 11, 2023
PR-URL: nodejs#48779
Fixes: nodejs#48778
Fixes: nodejs#48516
Refs: nodejs#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
targos pushed a commit that referenced this issue Nov 23, 2023
PR-URL: #48779
Backport-PR-URL: #50669
Fixes: #48778
Fixes: #48516
Refs: #46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
PR-URL: nodejs/node#48779
Backport-PR-URL: nodejs/node#50669
Fixes: nodejs/node#48778
Fixes: nodejs/node#48516
Refs: nodejs/node#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
PR-URL: nodejs/node#48779
Backport-PR-URL: nodejs/node#50669
Fixes: nodejs/node#48778
Fixes: nodejs/node#48516
Refs: nodejs/node#46402
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants