-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: Add support for pjson cache and main without extensions #18728
Changes from all commits
f14420e
adfe1ac
ebdb3cb
5855e5c
05c9941
2d6d4e6
a06db2d
081aab4
ed9df8a
8b4d85f
4468e2c
b4dc802
9f7aabd
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 |
---|---|---|
@@ -1,17 +1,54 @@ | ||
'use strict'; | ||
|
||
const { | ||
setImportModuleDynamicallyCallback, | ||
setInitializeImportMetaObjectCallback | ||
} = internalBinding('module_wrap'); | ||
|
||
const { getURLFromFilePath } = require('internal/url'); | ||
const Loader = require('internal/loader/Loader'); | ||
const path = require('path'); | ||
const { URL } = require('url'); | ||
|
||
function normalizeReferrerURL(referrer) { | ||
if (typeof referrer === 'string' && path.isAbsolute(referrer)) { | ||
return getURLFromFilePath(referrer).href; | ||
} | ||
return new URL(referrer).href; | ||
} | ||
|
||
function initializeImportMetaObject(wrap, meta) { | ||
meta.url = wrap.url; | ||
} | ||
|
||
function setupModules() { | ||
let loaderResolve; | ||
exports.loaderPromise = new Promise((resolve, reject) => { | ||
loaderResolve = resolve; | ||
}); | ||
|
||
exports.ESMLoader = undefined; | ||
|
||
exports.setup = function() { | ||
setInitializeImportMetaObjectCallback(initializeImportMetaObject); | ||
} | ||
|
||
module.exports = { | ||
setup: setupModules | ||
let ESMLoader = new Loader(); | ||
const loaderPromise = (async () => { | ||
const userLoader = process.binding('config').userLoader; | ||
if (userLoader) { | ||
const hooks = await ESMLoader.import( | ||
userLoader, getURLFromFilePath(`${process.cwd()}/`).href); | ||
ESMLoader = new Loader(); | ||
ESMLoader.hook(hooks); | ||
exports.ESMLoader = ESMLoader; | ||
} | ||
return ESMLoader; | ||
})(); | ||
loaderResolve(loaderPromise); | ||
|
||
setImportModuleDynamicallyCallback(async (referrer, specifier) => { | ||
const loader = await loaderPromise; | ||
return loader.import(specifier, normalizeReferrerURL(referrer)); | ||
}); | ||
|
||
exports.ESMLoader = ESMLoader; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,6 @@ | |
const NativeModule = require('native_module'); | ||
const util = require('util'); | ||
const { decorateErrorStack } = require('internal/util'); | ||
const internalModule = require('internal/module'); | ||
const { getURLFromFilePath } = require('internal/url'); | ||
const vm = require('vm'); | ||
const assert = require('assert').ok; | ||
|
@@ -35,6 +34,7 @@ const { | |
internalModuleReadJSON, | ||
internalModuleStat | ||
} = process.binding('fs'); | ||
const internalModule = require('internal/module'); | ||
const preserveSymlinks = !!process.binding('config').preserveSymlinks; | ||
const experimentalModules = !!process.binding('config').experimentalModules; | ||
|
||
|
@@ -43,10 +43,9 @@ const errors = require('internal/errors'); | |
module.exports = Module; | ||
|
||
// these are below module.exports for the circular reference | ||
const Loader = require('internal/loader/Loader'); | ||
const internalESModule = require('internal/process/modules'); | ||
const ModuleJob = require('internal/loader/ModuleJob'); | ||
const createDynamicModule = require('internal/loader/CreateDynamicModule'); | ||
let ESMLoader; | ||
|
||
function stat(filename) { | ||
filename = path.toNamespacedPath(filename); | ||
|
@@ -447,7 +446,6 @@ Module._resolveLookupPaths = function(request, parent, newReturn) { | |
return (newReturn ? parentDir : [id, parentDir]); | ||
}; | ||
|
||
|
||
// Check the cache for the requested file. | ||
// 1. If a module already exists in the cache: return its exports object. | ||
// 2. If the module is native: call `NativeModule.require()` with the | ||
|
@@ -460,22 +458,10 @@ Module._load = function(request, parent, isMain) { | |
debug('Module._load REQUEST %s parent: %s', request, parent.id); | ||
} | ||
|
||
if (isMain && experimentalModules) { | ||
(async () => { | ||
// loader setup | ||
if (!ESMLoader) { | ||
ESMLoader = new Loader(); | ||
const userLoader = process.binding('config').userLoader; | ||
if (userLoader) { | ||
ESMLoader.isMain = false; | ||
const hooks = await ESMLoader.import(userLoader); | ||
ESMLoader = new Loader(); | ||
ESMLoader.hook(hooks); | ||
} | ||
} | ||
Loader.registerImportDynamicallyCallback(ESMLoader); | ||
await ESMLoader.import(getURLFromFilePath(request).pathname); | ||
})() | ||
if (experimentalModules && isMain) { | ||
internalESModule.loaderPromise.then((loader) => { | ||
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'm not a fan of our error handling logic here of 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. @benjamingr that was done because unhandled rejection gave even worse error messages to the user. 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. @bmeck then we should either fix the unhandled rejection error messages or If you can point me towards related reading I promise to read it :) 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. related reading?? 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. For adding the behavior for error messages 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. |
||
return loader.import(getURLFromFilePath(request).pathname); | ||
}) | ||
.catch((e) => { | ||
decorateErrorStack(e); | ||
console.error(e); | ||
|
@@ -578,7 +564,8 @@ Module.prototype.load = function(filename) { | |
Module._extensions[extension](this, filename); | ||
this.loaded = true; | ||
|
||
if (ESMLoader) { | ||
if (experimentalModules) { | ||
const ESMLoader = internalESModule.ESMLoader; | ||
const url = getURLFromFilePath(filename); | ||
const urlString = `${url}`; | ||
const exports = this.exports; | ||
|
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.
Reading this again, can you explain why removing the
if (this.main)
withoutif (parentURL)
is valid?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.
Because we clear
_breakFirstLine
it's guaranteed to only trigger for the first module loaded.