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

Commit 2ace497

Browse files
MylesBorinsMylesBorins
committed
modules: remove file extension and directory resolving for esm
this build on top of the limitations of the package name map proposal. It removes file extensions and directory resolution in the resolver. This is only currently limited for local development. When resolving dependencies file extension and directory resolution will still work. Refs: https://github.com/domenic/package-name-maps
1 parent 0ade10d commit 2ace497

37 files changed

Lines changed: 106 additions & 88 deletions

‎doc/api/esm.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ property:
5454

5555
## Notable differences between `import` and `require`
5656

57+
### Mandatory file extensions
58+
59+
You must provide a file extension when using the `import` keyword.
60+
61+
### No importing directories
62+
63+
There is no support for importing directories.
64+
5765
### No NODE_PATH
5866

5967
`NODE_PATH` is not part of resolving `import` specifiers. Please use symlinks
@@ -78,8 +86,8 @@ Modules will be loaded multiple times if the `import` specifier used to resolve
7886
them have a different query or fragment.
7987

8088
```js
81-
import './foo?query=1'; // loads ./foo with query of "?query=1"
82-
import './foo?query=2'; // loads ./foo with query of "?query=2"
89+
import './foo.mjs?query=1'; // loads ./foo.mjs with query of "?query=1"
90+
import './foo.mjs?query=2'; // loads ./foo.mjs with query of "?query=2"
8391
```
8492

8593
For now, only modules using the `file:` protocol can be loaded.

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ const {
4242
const preserveSymlinks = !!process.binding('config').preserveSymlinks;
4343
const preserveSymlinksMain = !!process.binding('config').preserveSymlinksMain;
4444
const experimentalModules = !!process.binding('config').experimentalModules;
45+
const hasLoader = !!process.binding('config').userLoader;
4546

4647
const {
4748
ERR_INVALID_ARG_TYPE,
@@ -733,7 +734,11 @@ if (experimentalModules) {
733734
// bootstrap main module.
734735
Module.runMain = function() {
735736
// Load the main module--the command line argument.
736-
if (experimentalModules) {
737+
const base = path.basename(process.argv[1]);
738+
const ext = path.extname(base);
739+
const isESM = ext === '.mjs';
740+
741+
if (experimentalModules && (isESM || hasLoader)) {
737742
if (asyncESM === undefined) lazyLoadESM();
738743
asyncESM.loaderPromise.then((loader) => {
739744
return loader.import(pathToFileURL(process.argv[1]).pathname);

‎src/module_wrap.cc‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,12 @@ enum ResolveExtensionsOptions {
552552
ONLY_VIA_EXTENSIONS
553553
};
554554

555+
inline bool ResolvesToFile(const URL& search) {
556+
std::string filePath = search.ToFilePath();
557+
Maybe<uv_file> check = CheckFile(filePath);
558+
return !check.IsNothing();
559+
}
560+
555561
template <ResolveExtensionsOptions options>
556562
Maybe<URL> ResolveExtensions(const URL& search) {
557563
if (options == TRY_EXACT_NAME) {
@@ -589,10 +595,20 @@ Maybe<URL> ResolveMain(Environment* env, const URL& search) {
589595
pjson.has_main == HasMain::No) {
590596
return Nothing<URL>();
591597
}
592-
if (!ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) {
593-
return Resolve(env, "./" + pjson.main, search, IgnoreMain);
598+
599+
URL resolved;
600+
if (ShouldBeTreatedAsRelativeOrAbsolutePath(pjson.main)) {
601+
resolved = URL(pjson.main, search);
602+
} else {
603+
resolved = URL("./" + pjson.main, search);
604+
}
605+
Maybe<URL> file = ResolveExtensions<TRY_EXACT_NAME>(resolved);
606+
if (!file.IsNothing())
607+
return file;
608+
if (pjson.main.back() != '/') {
609+
resolved = URL(pjson.main + "/", search);
594610
}
595-
return Resolve(env, pjson.main, search, IgnoreMain);
611+
return ResolveIndex(resolved);
596612
}
597613

598614
Maybe<URL> ResolveModule(Environment* env,
@@ -603,7 +619,7 @@ Maybe<URL> ResolveModule(Environment* env,
603619
do {
604620
dir = parent;
605621
Maybe<URL> check =
606-
Resolve(env, "./node_modules/" + specifier, dir, CheckMain);
622+
Resolve(env, "./node_modules/" + specifier, dir);
607623
if (!check.IsNothing()) {
608624
const size_t limit = specifier.find('/');
609625
const size_t spec_len =
@@ -623,23 +639,11 @@ Maybe<URL> ResolveModule(Environment* env,
623639
return Nothing<URL>();
624640
}
625641

626-
Maybe<URL> ResolveDirectory(Environment* env,
627-
const URL& search,
628-
PackageMainCheck check_pjson_main) {
629-
if (check_pjson_main) {
630-
Maybe<URL> main = ResolveMain(env, search);
631-
if (!main.IsNothing())
632-
return main;
633-
}
634-
return ResolveIndex(search);
635-
}
636-
637642
} // anonymous namespace
638643

639644
Maybe<URL> Resolve(Environment* env,
640645
const std::string& specifier,
641-
const URL& base,
642-
PackageMainCheck check_pjson_main) {
646+
const URL& base) {
643647
URL pure_url(specifier);
644648
if (!(pure_url.flags() & URL_FLAGS_FAILED)) {
645649
// just check existence, without altering
@@ -654,13 +658,9 @@ Maybe<URL> Resolve(Environment* env,
654658
}
655659
if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
656660
URL resolved(specifier, base);
657-
Maybe<URL> file = ResolveExtensions<TRY_EXACT_NAME>(resolved);
658-
if (!file.IsNothing())
659-
return file;
660-
if (specifier.back() != '/') {
661-
resolved = URL(specifier + "/", base);
662-
}
663-
return ResolveDirectory(env, resolved, check_pjson_main);
661+
if (ResolvesToFile(resolved))
662+
return Just(resolved);
663+
return Nothing<URL>();
664664
} else {
665665
return ResolveModule(env, specifier, base);
666666
}

‎src/module_wrap.h‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,9 @@
1212
namespace node {
1313
namespace loader {
1414

15-
enum PackageMainCheck : bool {
16-
CheckMain = true,
17-
IgnoreMain = false
18-
};
19-
2015
v8::Maybe<url::URL> Resolve(Environment* env,
2116
const std::string& specifier,
22-
const url::URL& base,
23-
PackageMainCheck read_pkg_json = CheckMain);
17+
const url::URL& base);
2418

2519
class ModuleWrap : public BaseObject {
2620
public:

‎test/es-module/test-esm-basic-imports.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Flags: --experimental-modules
2-
import '../common';
2+
/* eslint-disable node-core/required-modules */
3+
import '../common/index.mjs';
34
import assert from 'assert';
45
import ok from '../fixtures/es-modules/test-esm-ok.mjs';
56
import okShebang from './test-esm-shebang.mjs';

‎test/es-module/test-esm-cyclic-dynamic-import.mjs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
// Flags: --experimental-modules
2-
import '../common';
3-
import('./test-esm-cyclic-dynamic-import');
2+
/* eslint-disable node-core/required-modules */
3+
import '../common/index.mjs';
4+
import('./test-esm-cyclic-dynamic-import.mjs');

‎test/es-module/test-esm-double-encoding.mjs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Flags: --experimental-modules
2-
import '../common';
2+
/* eslint-disable node-core/required-modules */
3+
import '../common/index.mjs';
34

45
// Assert we can import files with `%` in their pathname.
56

6-
import '../fixtures/es-modules/test-esm-double-encoding-native%2520.js';
7+
import '../fixtures/es-modules/test-esm-double-encoding-native%2520.mjs';

‎test/es-module/test-esm-encoded-path.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Flags: --experimental-modules
2-
import '../common';
2+
/* eslint-disable node-core/required-modules */
3+
import '../common/index.mjs';
34
import assert from 'assert';
45
// ./test-esm-ok.mjs
56
import ok from '../fixtures/es-modules/test-%65%73%6d-ok.mjs';

‎test/es-module/test-esm-forbidden-globals.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Flags: --experimental-modules
2-
import '../common';
2+
/* eslint-disable node-core/required-modules */
3+
import '../common/index.mjs';
34

45
// eslint-disable-next-line no-undef
56
if (typeof arguments !== 'undefined') {

‎test/es-module/test-esm-import-meta.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Flags: --experimental-modules
2+
/* eslint-disable node-core/required-modules */
23

3-
import '../common';
4+
import '../common/index.mjs';
45
import assert from 'assert';
56

67
assert.strictEqual(Object.getPrototypeOf(import.meta), null);

0 commit comments

Comments
 (0)