-
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 6 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,51 +1,21 @@ | ||
'use strict'; | ||
|
||
const path = require('path'); | ||
const { getURLFromFilePath, URL } = require('internal/url'); | ||
const errors = require('internal/errors'); | ||
|
||
const ModuleMap = require('internal/loader/ModuleMap'); | ||
const ModuleJob = require('internal/loader/ModuleJob'); | ||
const defaultResolve = require('internal/loader/DefaultResolve'); | ||
const createDynamicModule = require('internal/loader/CreateDynamicModule'); | ||
const translators = require('internal/loader/Translators'); | ||
const { setImportModuleDynamicallyCallback } = internalBinding('module_wrap'); | ||
|
||
const FunctionBind = Function.call.bind(Function.prototype.bind); | ||
|
||
const debug = require('util').debuglog('esm'); | ||
|
||
// Returns a file URL for the current working directory. | ||
function getURLStringForCwd() { | ||
try { | ||
return getURLFromFilePath(`${process.cwd()}/`).href; | ||
} catch (e) { | ||
e.stack; | ||
// If the current working directory no longer exists. | ||
if (e.code === 'ENOENT') { | ||
return undefined; | ||
} | ||
throw e; | ||
} | ||
} | ||
|
||
function normalizeReferrerURL(referrer) { | ||
if (typeof referrer === 'string' && path.isAbsolute(referrer)) { | ||
return getURLFromFilePath(referrer).href; | ||
} | ||
return new URL(referrer).href; | ||
} | ||
|
||
/* A Loader instance is used as the main entry point for loading ES modules. | ||
* Currently, this is a singleton -- there is only one used for loading | ||
* the main module and everything in its dependency graph. */ | ||
class Loader { | ||
constructor(base = getURLStringForCwd()) { | ||
if (typeof base !== 'string') | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'base', 'string'); | ||
|
||
this.base = base; | ||
this.isMain = true; | ||
|
||
constructor() { | ||
// methods which translate input code or other information | ||
// into es modules | ||
this.translators = translators; | ||
|
@@ -71,8 +41,9 @@ class Loader { | |
this._dynamicInstantiate = undefined; | ||
} | ||
|
||
async resolve(specifier, parentURL = this.base) { | ||
if (typeof parentURL !== 'string') | ||
async resolve(specifier, parentURL) { | ||
const isMain = parentURL === undefined; | ||
if (!isMain && typeof parentURL !== 'string') | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'parentURL', 'string'); | ||
|
||
const { url, format } = | ||
|
@@ -93,7 +64,7 @@ class Loader { | |
return { url, format }; | ||
} | ||
|
||
async import(specifier, parent = this.base) { | ||
async import(specifier, parent) { | ||
const job = await this.getModuleJob(specifier, parent); | ||
const module = await job.run(); | ||
return module.namespace(); | ||
|
@@ -107,7 +78,7 @@ class Loader { | |
this._dynamicInstantiate = FunctionBind(dynamicInstantiate, null); | ||
} | ||
|
||
async getModuleJob(specifier, parentURL = this.base) { | ||
async getModuleJob(specifier, parentURL) { | ||
const { url, format } = await this.resolve(specifier, parentURL); | ||
let job = this.moduleMap.get(url); | ||
if (job !== undefined) | ||
|
@@ -134,24 +105,16 @@ class Loader { | |
} | ||
|
||
let inspectBrk = false; | ||
if (this.isMain) { | ||
if (process._breakFirstLine) { | ||
delete process._breakFirstLine; | ||
inspectBrk = true; | ||
} | ||
this.isMain = false; | ||
if (process._breakFirstLine) { | ||
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. Reading this again, can you explain why removing 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. Because we clear |
||
delete process._breakFirstLine; | ||
inspectBrk = true; | ||
} | ||
job = new ModuleJob(this, url, loaderInstance, inspectBrk); | ||
this.moduleMap.set(url, job); | ||
return job; | ||
} | ||
|
||
static registerImportDynamicallyCallback(loader) { | ||
setImportModuleDynamicallyCallback(async (referrer, specifier) => { | ||
return loader.import(specifier, normalizeReferrerURL(referrer)); | ||
}); | ||
} | ||
} | ||
|
||
Object.setPrototypeOf(Loader.prototype, null); | ||
|
||
module.exports = Loader; |
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 () => { | ||
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. This isn't a great pattern and errors here are suppressed unless explicitly asked for |
||
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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -608,6 +608,13 @@ class Environment { | |
|
||
std::unordered_multimap<int, loader::ModuleWrap*> module_map; | ||
|
||
struct PackageConfig { | ||
bool exists; | ||
bool has_main; | ||
std::string main; | ||
}; | ||
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. PackageConfig should probably live 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. Would still be nice if these were const. 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. As far as I can tell getting this to work would mean inlining 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. (as in 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.
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. The first case didn't work unfortunately. In the second case it would need to be a 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 managed to get the |
||
std::unordered_map<std::string, PackageConfig> package_json_cache; | ||
|
||
inline double* heap_statistics_buffer() const; | ||
inline void set_heap_statistics_buffer(double* pointer); | ||
|
||
|
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.
Just wondering, why is this wrapping of
${url}
required?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.
This is a URL -> string serialization convention in this code.