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

[WIP] Support requiring .mjs files #30891

Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 39 additions & 27 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ const {
ERR_REQUIRE_ESM
} = require('internal/errors').codes;
const { validateString } = require('internal/validators');
const { promiseWait } = internalBinding('task_queue');
const pendingDeprecation = getOptionValue('--pending-deprecation');

module.exports = {
Expand Down Expand Up @@ -1038,36 +1039,34 @@ Module.prototype.load = function(filename) {
this.paths = Module._nodeModulePaths(path.dirname(filename));

const extension = findLongestRegisteredExtension(filename);
// allow .mjs to be overridden
if (filename.endsWith('.mjs') && !Module._extensions['.mjs']) {
throw new ERR_REQUIRE_ESM(filename);
}
Module._extensions[extension](this, filename);
this.loaded = true;

const ESMLoader = asyncESM.ESMLoader;
const url = `${pathToFileURL(filename)}`;
const module = ESMLoader.moduleMap.get(url);
// Create module entry at load time to snapshot exports correctly
const exports = this.exports;
// Called from cjs translator
if (module !== undefined && module.module !== undefined) {
if (module.module.getStatus() >= kInstantiated)
module.module.setExport('default', exports);
} else {
// Preemptively cache
// We use a function to defer promise creation for async hooks.
ESMLoader.moduleMap.set(
url,
// Module job creation will start promises.
// We make it a function to lazily trigger those promises
// for async hooks compatibility.
() => new ModuleJob(ESMLoader, url, () =>
new ModuleWrap(url, undefined, ['default'], function() {
this.setExport('default', exports);
})
, false /* isMain */, false /* inspectBrk */)
);
if (extension !== '.mjs') {
const ESMLoader = asyncESM.ESMLoader;
const url = `${pathToFileURL(filename)}`;
const module = ESMLoader.moduleMap.get(url);
// Create module entry at load time to snapshot exports correctly
const exports = this.exports;
// Called from cjs translator
if (module !== undefined && module.module !== undefined) {
if (module.module.getStatus() >= kInstantiated)
module.module.setExport('default', exports);
} else {
// Preemptively cache
// We use a function to defer promise creation for async hooks.
ESMLoader.moduleMap.set(
url,
// Module job creation will start promises.
// We make it a function to lazily trigger those promises
// for async hooks compatibility.
() => new ModuleJob(ESMLoader, url, () =>
new ModuleWrap(url, undefined, ['default'], function() {
this.setExport('default', exports);
})
, false /* isMain */, false /* inspectBrk */)
);
}
}
};

Expand Down Expand Up @@ -1246,6 +1245,19 @@ Module._extensions['.node'] = function(module, filename) {
return process.dlopen(module, path.toNamespacedPath(filename));
};

Module._extensions['.mjs'] = function(module, filename) {
const ESMLoader = asyncESM.ESMLoader;
const url = `${pathToFileURL(filename)}`;
const job = ESMLoader.getModuleJobWorker(url, 'module');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this just skips the esm resolver entirely? this seems like a rather dangerous divergence.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because we already resolved the specifier, in order to know we needed the esm loader. The cjs resolver and the esm resolver are supposed to resolve the same specifier to the same thing, so it should be fine. This is part of why I kept saying it really should only be considered to be one loader.

/* So long as builtin esm resolve is sync, this will complete sync
* (if the loader is extensible, an async loader will be have an observable
* effect here)
*/
const instantiated = promiseWait(job.instantiate());
module.exports = instantiated.getNamespace();
instantiated.evaluate(-1, false);
}

function createRequireFromPath(filename) {
// Allow a directory to be passed as the filename
const trailingSlash =
Expand Down
8 changes: 6 additions & 2 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ class Loader {
}
}

async getModuleJob(specifier, parentURL) {
const { url, format } = await this.resolve(specifier, parentURL);
getModuleJobWorker(url, format, parentURL) {
let job = this.moduleMap.get(url);
// CommonJS will set functions for lazy job evaluation.
if (typeof job === 'function')
Expand Down Expand Up @@ -188,6 +187,11 @@ class Loader {
this.moduleMap.set(url, job);
return job;
}

async getModuleJob(specifier, parentURL) {
const { url, format } = await this.resolve(specifier, parentURL);
return this.getModuleJobWorker(url, format, parentURL);
}
}

ObjectSetPrototypeOf(Loader.prototype, null);
Expand Down
41 changes: 41 additions & 0 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,45 @@ static void SetPromiseRejectCallback(
env->set_promise_reject_callback(args[0].As<Function>());
}

/**
* Immediately unwraps a promise into a return value or throw, if possible
* If not, runs the event loop and microtask queue until it is unwrapable.
*/
static void PromiseWait(const FunctionCallbackInfo<Value>& args) {
if (!args[0]->IsPromise()) {
args.GetReturnValue().Set(args[0]);
return;
}
v8::Local<v8::Promise> promise = args[0].As<v8::Promise>();
if (promise->State() == v8::Promise::kFulfilled) {
args.GetReturnValue().Set(promise->Result());
return;
}
Isolate* isolate = args.GetIsolate();
if (promise->State() == v8::Promise::kRejected) {
isolate->ThrowException(promise->Result());
return;
}

Environment* env = Environment::GetCurrent(args);

uv_loop_t* loop = env->event_loop();
int state = promise->State();
while (state == v8::Promise::kPending) {
isolate->RunMicrotasks();
if (uv_loop_alive(loop) && promise->State() == v8::Promise::kPending) {
uv_run(loop, UV_RUN_ONCE);
}
state = promise->State();
}

if (promise->State() == v8::Promise::kRejected) {
isolate->ThrowException(promise->Result());
return;
}
args.GetReturnValue().Set(promise->Result());
}

static void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
Expand All @@ -145,6 +184,8 @@ static void Initialize(Local<Object> target,
env->SetMethod(target,
"setPromiseRejectCallback",
SetPromiseRejectCallback);

env->SetMethod(target, "promiseWait", PromiseWait);
}

} // namespace task_queue
Expand Down
8 changes: 1 addition & 7 deletions test/parallel/test-require-mjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,4 @@
require('../common');
const assert = require('assert');

assert.throws(
() => require('../fixtures/es-modules/test-esm-ok.mjs'),
{
message: /Must use import to load ES Module/,
code: 'ERR_REQUIRE_ESM'
}
);
assert.equal(require('../fixtures/es-modules/test-esm-ok.mjs').default, true);