Skip to content
This repository was archived by the owner on Apr 16, 2020. It is now read-only.

--es-module-specifier-resolution flag #48

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,6 @@ READ_PACKAGE_JSON(_packageURL_)
[Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md
[dynamic instantiate hook]: #esm_dynamic_instantiate_hook
[`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename
[`import.meta.url`]: esm.html#importmeta
[`import()`]: esm.html#import-expressions
[`import.meta.url`]: #esm_import_meta
[`import()`]: #esm_import-expressions
[ecmascript-modules implementation]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md
120 changes: 88 additions & 32 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ using v8::String;
using v8::Undefined;
using v8::Value;

static const char* const EXTENSIONS[] = {
".mjs",
".cjs",
".js",
".json",
".node"
};

ModuleWrap::ModuleWrap(Environment* env,
Local<Object> object,
Local<Module> module,
Expand Down Expand Up @@ -667,13 +675,57 @@ Maybe<URL> LegacyMainResolve(const URL& pjson_url,
return Nothing<URL>();
}

enum ResolveExtensionsOptions {
TRY_EXACT_NAME,
ONLY_VIA_EXTENSIONS
};

template <ResolveExtensionsOptions options>
Maybe<URL> ResolveExtensions(const URL& search) {
if (options == TRY_EXACT_NAME) {
if (FileExists(search)) {
return Just(search);
}
}

for (const char* extension : EXTENSIONS) {
URL guess(search.path() + extension, &search);
if (FileExists(guess)) {
return Just(guess);
}
}

return Nothing<URL>();
}

inline Maybe<URL> ResolveIndex(const URL& search) {
return ResolveExtensions<ONLY_VIA_EXTENSIONS>(URL("index", search));
}

Maybe<URL> FinalizeResolution(Environment* env,
const URL& resolved,
const URL& base,
bool check_exists) {
const std::string& path = resolved.ToFilePath();
const URL& resolved,
const URL& base) {
if (env->options()->es_module_specifier_resolution == "node") {
Maybe<URL> file = ResolveExtensions<TRY_EXACT_NAME>(resolved);
if (!file.IsNothing()) {
return file;
}
if (resolved.path().back() != '/') {
file = ResolveIndex(URL(resolved.path() + "/", &base));
} else {
file = ResolveIndex(resolved);
}
if (!file.IsNothing()) {
return file;
}
std::string msg = "Cannot find module '" + resolved.path() +
"' imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
return Nothing<URL>();
}

if (check_exists && CheckDescriptorAtPath(path) != FILE) {
const std::string& path = resolved.ToFilePath();
if (CheckDescriptorAtPath(path) != FILE) {
std::string msg = "Cannot find module '" + path +
"' imported from " + base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
Expand All @@ -684,32 +736,36 @@ Maybe<URL> FinalizeResolution(Environment* env,
}

Maybe<URL> PackageMainResolve(Environment* env,
const URL& pjson_url,
const PackageConfig& pcfg,
const URL& base) {
if (pcfg.exists == Exists::No || (
pcfg.esm == IsESM::Yes && pcfg.has_main == HasMain::No)) {
std::string msg = "Cannot find main entry point for '" +
URL(".", pjson_url).ToFilePath() + "' imported from " +
base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
return Nothing<URL>();
}
if (pcfg.has_main == HasMain::Yes &&
pcfg.main.substr(pcfg.main.length() - 4, 4) == ".mjs") {
return FinalizeResolution(env, URL(pcfg.main, pjson_url), base, true);
}
if (pcfg.esm == IsESM::Yes &&
pcfg.main.substr(pcfg.main.length() - 3, 3) == ".js") {
return FinalizeResolution(env, URL(pcfg.main, pjson_url), base, true);
}

Maybe<URL> resolved = LegacyMainResolve(pjson_url, pcfg);
// Legacy main resolution error
if (resolved.IsNothing()) {
return Nothing<URL>();
const URL& pjson_url,
const PackageConfig& pcfg,
const URL& base) {
if (pcfg.exists == Exists::Yes) {
if (pcfg.has_main == HasMain::Yes) {
URL resolved(pcfg.main, pjson_url);
const std::string& path = resolved.ToFilePath();
if (CheckDescriptorAtPath(path) == FILE) {
return Just(resolved);
}
}
if (env->options()->es_module_specifier_resolution == "node") {
if (pcfg.has_main == HasMain::Yes) {
return FinalizeResolution(env, URL(pcfg.main, pjson_url), base);
} else {
return FinalizeResolution(env, URL("index", pjson_url), base);
}
}
if (pcfg.esm == IsESM::No) {
Maybe<URL> resolved = LegacyMainResolve(pjson_url, pcfg);
if (!resolved.IsNothing()) {
return resolved;
}
}
}
return resolved;
std::string msg = "Cannot find main entry point for '" +
URL(".", pjson_url).ToFilePath() + "' imported from " +
base.ToFilePath();
node::THROW_ERR_MODULE_NOT_FOUND(env, msg.c_str());
return Nothing<URL>();
}

Maybe<URL> PackageResolve(Environment* env,
Expand Down Expand Up @@ -759,7 +815,7 @@ Maybe<URL> PackageResolve(Environment* env,
if (!pkg_subpath.length()) {
return PackageMainResolve(env, pjson_url, *pcfg.FromJust(), base);
} else {
return FinalizeResolution(env, URL(pkg_subpath, pjson_url), base, true);
return FinalizeResolution(env, URL(pkg_subpath, pjson_url), base);
}
CHECK(false);
// Cross-platform root check.
Expand Down Expand Up @@ -789,7 +845,7 @@ Maybe<URL> Resolve(Environment* env,
return PackageResolve(env, specifier, base);
}
}
return FinalizeResolution(env, resolved, base, true);
return FinalizeResolution(env, resolved, base);
}

void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& args) {
Expand Down
5 changes: 5 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"custom loader",
&EnvironmentOptions::userland_loader,
kAllowedInEnvironment);
AddOption("--es-module-specifier-resolution",
"Select extension resolution algorithm for es modules; "
"either 'explicit' (default) or 'node'",
&EnvironmentOptions::es_module_specifier_resolution,
kAllowedInEnvironment);
AddOption("--no-deprecation",
"silence deprecation warnings",
&EnvironmentOptions::no_deprecation,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class EnvironmentOptions : public Options {
public:
bool abort_on_uncaught_exception = false;
bool experimental_modules = false;
std::string es_module_specifier_resolution = "explicit";
std::string module_type;
std::string experimental_policy;
bool experimental_repl_await = false;
Expand Down
12 changes: 0 additions & 12 deletions test/es-module/test-esm-package-scope.mjs

This file was deleted.

35 changes: 35 additions & 0 deletions test/es-module/test-esm-specifiers.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Flags: --experimental-modules --es-module-specifier-resolution=node
import { mustNotCall } from '../common';
import assert from 'assert';

// commonJS index.js
import commonjs from '../fixtures/es-module-specifiers/package-type-commonjs';
// esm index.js
import module from '../fixtures/es-module-specifiers/package-type-module';
// notice the trailing slash
import success, { explicit, implicit, implicitModule, getImplicitCommonjs }
from '../fixtures/es-module-specifiers/';

assert.strictEqual(commonjs, 'commonjs');
assert.strictEqual(module, 'module');
assert.strictEqual(success, 'success');
assert.strictEqual(explicit, 'esm');
assert.strictEqual(implicit, 'esm');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So with this change, from the prior loader, .mjs would be resolved before .js? would .cjs resolve before .mjs?

assert.strictEqual(implicitModule, 'esm');

async function main() {
try {
await import('../fixtures/es-module-specifiers/do-not-exist.js');
} catch (e) {
// Files that do not exist should throw
assert.strictEqual(e.name, 'Error');
}
try {
await getImplicitCommonjs();
} catch (e) {
// Legacy loader cannot resolve .mjs automatically from main
assert.strictEqual(e.name, 'Error');
}
}

main().catch(mustNotCall);
10 changes: 10 additions & 0 deletions test/fixtures/es-module-specifiers/index.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import explicit from 'explicit-main';
import implicit from 'implicit-main';
import implicitModule from 'implicit-main-type-module';

function getImplicitCommonjs () {
return import('implicit-main-type-commonjs');
}

export {explicit, implicit, implicitModule, getImplicitCommonjs};
export default 'success';

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {b} from './b.mjs';
// import 'c.cjs';
import cjs from './c.cjs';
// proves cross boundary fun bits
import jsAsEsm from '../new-loader/a.js';
import jsAsEsm from '../package-type-module/a.js';

// named export from core
import {strictEqual, deepStrictEqual} from 'assert';
Expand All @@ -18,4 +18,4 @@ deepStrictEqual(cjs, {
three: 3
});

export default 'legacy-loader';
export default 'commonjs';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "commonjs"
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {b} from './b.mjs';
// import 'c.cjs';
import cjs from './c.cjs';
// import across boundaries
import jsAsCjs from '../legacy-loader/a.js'
import jsAsCjs from '../package-type-commonjs/a.js'

// named export from core
import {strictEqual, deepStrictEqual} from 'assert';
Expand All @@ -18,4 +18,4 @@ deepStrictEqual(cjs, {
three: 3
});

export default 'new-loader';
export default 'module';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
1 change: 1 addition & 0 deletions test/fixtures/es-module-specifiers/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
13 changes: 0 additions & 13 deletions test/fixtures/esm-package-scope/legacy-loader/package.json

This file was deleted.

13 changes: 0 additions & 13 deletions test/fixtures/esm-package-scope/new-loader/package.json

This file was deleted.