-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
loader: use default loader as cascaded loader in the in loader worker #47620
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
Changes from all commits
3707d00
4c0fbfd
bb939a5
954e02e
b691d14
36f9c62
3db9ee1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -91,14 +91,22 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) { | |||||||
throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); | ||||||||
} | ||||||||
|
||||||||
function initializeESM() { | ||||||||
// This is configured during pre-execution. Specifically it's set to true for | ||||||||
// the loader worker in internal/main/worker_thread.js. | ||||||||
let _isLoaderWorker = false; | ||||||||
function initializeESM(isLoaderWorker = false) { | ||||||||
_isLoaderWorker = isLoaderWorker; | ||||||||
initializeDefaultConditions(); | ||||||||
// Setup per-isolate callbacks that locate data or callbacks that we keep | ||||||||
// track of for different ESM modules. | ||||||||
setInitializeImportMetaObjectCallback(initializeImportMetaObject); | ||||||||
setImportModuleDynamicallyCallback(importModuleDynamicallyCallback); | ||||||||
} | ||||||||
|
||||||||
function isLoaderWorker() { | ||||||||
return _isLoaderWorker; | ||||||||
} | ||||||||
|
||||||||
async function initializeHooks() { | ||||||||
const customLoaderPaths = getOptionValue('--experimental-loader'); | ||||||||
|
||||||||
|
@@ -165,4 +173,6 @@ module.exports = { | |||||||
initializeHooks, | ||||||||
getDefaultConditions, | ||||||||
getConditionsSet, | ||||||||
loaderWorkerId: 'internal/modules/esm/worker', | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's definitely not a path (because the internal module loader doesn't use paths), I think calling it an ID is more correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. O.o it is literally the path to the file (but with CJS's whacked exclusion of the file extension) and is even consumed as such: node/lib/internal/modules/esm/hooks.js Line 498 in 3707d00
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not the path to a file, because internal modules are not in the FS, it's more correct to think of them as an offset in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it’s whatever we call the string that’s the input to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||
isLoaderWorker, | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: just make this a getter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would assume a function call would be slower than just mapping directly to a primitive? |
||||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
import { spawnPromisified } from '../common/index.mjs'; | ||
import * as fixtures from '../common/fixtures.mjs'; | ||
import assert from 'node:assert'; | ||
import { execPath } from 'node:process'; | ||
import { describe, it } from 'node:test'; | ||
|
||
describe('Worker threads do not spawn infinitely', { concurrency: true }, () => { | ||
it('should not trigger an infinite loop when using a loader exports no recognized hooks', async () => { | ||
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ | ||
'--no-warnings', | ||
'--experimental-loader', | ||
fixtures.fileURL('empty.js'), | ||
'--eval', | ||
'setTimeout(() => console.log("hello"),99)', | ||
]); | ||
|
||
assert.strictEqual(stderr, ''); | ||
assert.match(stdout, /^hello\r?\n$/); | ||
assert.strictEqual(code, 0); | ||
assert.strictEqual(signal, null); | ||
}); | ||
|
||
it('should support a CommonJS entry point and a loader that imports a CommonJS module', async () => { | ||
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ | ||
'--no-warnings', | ||
'--experimental-loader', | ||
fixtures.fileURL('es-module-loaders/loader-with-dep.mjs'), | ||
fixtures.path('print-delayed.js'), | ||
]); | ||
|
||
assert.strictEqual(stderr, ''); | ||
assert.match(stdout, /^delayed\r?\n$/); | ||
assert.strictEqual(code, 0); | ||
assert.strictEqual(signal, null); | ||
}); | ||
|
||
it('should support --require and --import along with using a loader written in CJS and CJS entry point', async () => { | ||
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ | ||
'--no-warnings', | ||
'--eval', | ||
'setTimeout(() => console.log("D"),99)', | ||
'--import', | ||
fixtures.fileURL('printC.js'), | ||
'--experimental-loader', | ||
fixtures.fileURL('printB.js'), | ||
'--require', | ||
fixtures.path('printA.js'), | ||
]); | ||
|
||
assert.strictEqual(stderr, ''); | ||
// The worker code should always run before the --import, but the console.log might arrive late. | ||
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/); | ||
assert.strictEqual(code, 0); | ||
assert.strictEqual(signal, null); | ||
}); | ||
|
||
it('should support --require and --import along with using a loader written in ESM and ESM entry point', async () => { | ||
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ | ||
'--no-warnings', | ||
'--require', | ||
fixtures.path('printA.js'), | ||
'--experimental-loader', | ||
'data:text/javascript,console.log("B")', | ||
aduh95 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'--import', | ||
fixtures.fileURL('printC.js'), | ||
'--input-type=module', | ||
'--eval', | ||
'setTimeout(() => console.log("D"),99)', | ||
]); | ||
|
||
assert.strictEqual(stderr, ''); | ||
// The worker code should always run before the --import, but the console.log might arrive late. | ||
assert.match(stdout, /^A\r?\nA\r?\n(B\r?\nC|C\r?\nB)\r?\nD\r?\n$/); | ||
assert.strictEqual(code, 0); | ||
assert.strictEqual(signal, null); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
setTimeout(() => { | ||
console.log('delayed'); | ||
}, 100); |
Uh oh!
There was an error while loading. Please reload this page.