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] always load the ESM loader #279
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import { createRequire } from 'module'; | ||
import { fileURLToPath } from 'url'; | ||
import { send } from './ipc.mjs'; | ||
|
||
const require = createRequire(import.meta.url); | ||
|
||
export async function getFormat(url, context, defaultGetFormat) { | ||
const getPackageType = require('get-package-type'); | ||
const filePath = fileURLToPath(url); | ||
|
||
send({ required: filePath }); | ||
try { | ||
return await defaultGetFormat(url, context, defaultGetFormat); | ||
} catch (err) { | ||
if (err.code === 'ERR_UNKNOWN_FILE_EXTENSION') { | ||
return { format: await getPackageType(filePath) }; | ||
} | ||
throw err; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,20 @@ | ||||||||||||||||||||||||||
import { createRequire } from 'module'; | ||||||||||||||||||||||||||
import { fileURLToPath } from 'url'; | ||||||||||||||||||||||||||
import { send } from './ipc.mjs'; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
const require = createRequire(import.meta.url); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export async function load(url, context, defaultLoad) { | ||||||||||||||||||||||||||
const getPackageType = require('get-package-type'); | ||||||||||||||||||||||||||
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 looks like this is only used in the unknown file extension case. Let's get it out of the critical path. |
||||||||||||||||||||||||||
const filePath = fileURLToPath(url); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
send({ required: filePath }); | ||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||
return await defaultLoad(url, context, defaultLoad); | ||||||||||||||||||||||||||
} catch (err) { | ||||||||||||||||||||||||||
if (err.code === 'ERR_UNKNOWN_FILE_EXTENSION') { | ||||||||||||||||||||||||||
return { format: await getPackageType(filePath), source: null }; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
throw err; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+12
to
+19
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.
Suggested change
|
||||||||||||||||||||||||||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
const { emitWarning } = process; | ||
|
||
process.emitWarning = (warning, ...args) => { | ||
if (args[0] === 'ExperimentalWarning') { | ||
return; | ||
} | ||
|
||
if (args[0] && typeof args[0] === 'object' && args[0].type === 'ExperimentalWarning') { | ||
return; | ||
} | ||
|
||
return emitWarning(warning, ...args); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,19 @@ | ||
const { dirname, extname } = require('path'); | ||
const childProcess = require('child_process'); | ||
const { sync: resolve } = require('resolve'); | ||
const getPackageType = require('get-package-type'); | ||
|
||
const { getConfig } = require('./cfg'); | ||
const hook = require('./hook'); | ||
const { relay, send } = require('./ipc'); | ||
const resolveMain = require('./resolve-main'); | ||
|
||
// Remove wrap.js from the argv array | ||
process.argv.splice(1, 1); | ||
|
||
const script = process.argv[1]; | ||
const { extensions, fork, vm } = getConfig(script); | ||
|
||
if (process.env.NODE_DEV_PRELOAD) { | ||
require(process.env.NODE_DEV_PRELOAD); | ||
} | ||
require('./suppress-experimental'); | ||
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 don't think we should be hiding warnings by default. 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. Although I can see now that it gets quite annoying. 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 was also trying to hold off on implementing this but I figure that this way is probably for the best since it was affecting the test output a lot! |
||
|
||
// We want to exit on SIGTERM, but defer to existing SIGTERM handlers. | ||
process.once('SIGTERM', () => process.listenerCount('SIGTERM') || process.exit(0)); | ||
|
@@ -26,7 +23,7 @@ if (fork) { | |
// too. We also need to relay messages about required files to the parent. | ||
const originalFork = childProcess.fork; | ||
childProcess.fork = (modulePath, args, options) => { | ||
const child = originalFork(__filename, [modulePath].concat(args), options); | ||
const child = originalFork(modulePath, args, options); | ||
relay(child); | ||
return child; | ||
}; | ||
|
@@ -48,7 +45,7 @@ process.on('uncaughtException', err => { | |
}); | ||
|
||
// Hook into require() and notify the parent process about required files | ||
hook(vm, module, required => send({ required })); | ||
hook(vm, required => send({ required })); | ||
|
||
// Check if a module is registered for this extension | ||
const main = resolveMain(script); | ||
|
@@ -66,6 +63,3 @@ if (typeof mod === 'object' && mod.name) { | |
} else if (typeof mod === 'string') { | ||
require(resolve(mod, { basedir })); | ||
} | ||
|
||
// Execute the wrapped script | ||
ext === 'mjs' || getPackageType.sync(main) === 'module' ? import(main) : require(main); |
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.
@bjornstar @kherock this crashes on any import of built in modules now, of which the URL is in the format
node:<identifier>
, e.g.node:path
: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.
I created an issue for this with a little test-case: #281