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

Disable default extensions only for relative resolution within package boundaries #4

Closed
wants to merge 3 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
12 changes: 10 additions & 2 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ property:

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

### Mandatory file extensions

You must provide a file extension when using the `import` keyword.

### No importing directories

There is no support for importing directories.

This comment was marked as off-topic.

### No NODE_PATH

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

```js
import './foo?query=1'; // loads ./foo with query of "?query=1"
import './foo?query=2'; // loads ./foo with query of "?query=2"
import './foo.mjs?query=1'; // loads ./foo.mjs with query of "?query=1"
import './foo.mjs?query=2'; // loads ./foo.mjs with query of "?query=2"
```

For now, only modules using the `file:` protocol can be loaded.
Expand Down
35 changes: 35 additions & 0 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,25 @@ const PackageConfig& GetPackageConfig(Environment* env,
return entry.first->second;
}

const PackageConfig& FindPackageConfig(Environment* env,
const URL& search) {
URL pjson_path("package.json", &search);
while (true) {
const PackageConfig& pkg_json =
GetPackageConfig(env, pjson_path.ToFilePath());
if (pkg_json.exists == Exists::Yes) {
return pkg_json;
}
URL last_pjson_path = pjson_path;
pjson_path = URL("../package.json", pjson_path);
// Terminates at root where ../package.json equals ../../package.json
// (can't just check "/package.json" for Windows support).
if (pjson_path.path().length() == last_pjson_path.path().length()) {
return pkg_json;
}
}
}

enum ResolveExtensionsOptions {
TRY_EXACT_NAME,
ONLY_VIA_EXTENSIONS
Expand Down Expand Up @@ -654,6 +673,22 @@ Maybe<URL> Resolve(Environment* env,
}
if (ShouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
URL resolved(specifier, base);

// if resolving a relative path in the same package as the parent,
// don't support automatic extensions or directory resolution
if (specifier.front() != '/') {
const PackageConfig& parentPjson = FindPackageConfig(env, base);
const PackageConfig& pjson = FindPackageConfig(env, resolved);
if (&parentPjson == &pjson) {
// just check existence, without altering
Maybe<uv_file> check = CheckFile(resolved.ToFilePath());
if (check.IsNothing()) {
return Nothing<URL>();
}
return Just(resolved);
}
}

Maybe<URL> file = ResolveExtensions<TRY_EXACT_NAME>(resolved);
if (!file.IsNothing())
return file;
Expand Down
3 changes: 2 additions & 1 deletion test/es-module/test-esm-basic-imports.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';
import ok from '../fixtures/es-modules/test-esm-ok.mjs';
import okShebang from './test-esm-shebang.mjs';
Expand Down
5 changes: 3 additions & 2 deletions test/es-module/test-esm-cyclic-dynamic-import.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --experimental-modules
import '../common';
import('./test-esm-cyclic-dynamic-import');
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import('./test-esm-cyclic-dynamic-import.mjs');
5 changes: 3 additions & 2 deletions test/es-module/test-esm-double-encoding.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';

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

import '../fixtures/es-modules/test-esm-double-encoding-native%2520.js';
import '../fixtures/es-modules/test-esm-double-encoding-native%2520.mjs';
3 changes: 2 additions & 1 deletion test/es-module/test-esm-encoded-path.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';
// ./test-esm-ok.mjs
import ok from '../fixtures/es-modules/test-%65%73%6d-ok.mjs';
Expand Down
3 changes: 2 additions & 1 deletion test/es-module/test-esm-forbidden-globals.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';

// eslint-disable-next-line no-undef
if (typeof arguments !== 'undefined') {
Expand Down
3 changes: 2 additions & 1 deletion test/es-module/test-esm-import-meta.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */

import '../common';
import '../common/index.mjs';
import assert from 'assert';

assert.strictEqual(Object.getPrototypeOf(import.meta), null);
Expand Down
3 changes: 2 additions & 1 deletion test/es-module/test-esm-json.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';
import ok from '../fixtures/es-modules/test-esm-ok.mjs';
import json from '../fixtures/es-modules/json.json';
Expand Down
3 changes: 2 additions & 1 deletion test/es-module/test-esm-live-binding.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Flags: --experimental-modules
/* eslint-disable node-core/required-modules */

import '../common';
import '../common/index.mjs';
import assert from 'assert';

import fs, { readFile, readFileSync } from 'fs';
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-loader-invalid-format.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs
import { expectsError, mustCall } from '../common';
import { expectsError, mustCall } from '../common/index.mjs';
import assert from 'assert';

import('../fixtures/es-modules/test-esm-ok.mjs')
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-loader-invalid-url.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs
import { expectsError, mustCall } from '../common';
import { expectsError, mustCall } from '../common/index.mjs';
import assert from 'assert';

import('../fixtures/es-modules/test-esm-ok.mjs')
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/missing-dynamic-instantiate-hook.mjs

import { expectsError } from '../common';
import { expectsError } from '../common/index.mjs';

import('test').catch(expectsError({
code: 'ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK',
Expand Down
6 changes: 0 additions & 6 deletions test/es-module/test-esm-main-lookup.mjs

This file was deleted.

3 changes: 2 additions & 1 deletion test/es-module/test-esm-named-exports.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import { readFile } from 'fs';
import assert from 'assert';
import ok from '../fixtures/es-modules/test-esm-ok.mjs';
Expand Down
4 changes: 3 additions & 1 deletion test/es-module/test-esm-namespace.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */

import '../common/index.mjs';
import * as fs from 'fs';
import assert from 'assert';
import Module from 'module';
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-preserve-symlinks-not-found.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/not-found-assert-loader.mjs
/* eslint-disable node-core/required-modules */
import './not-found';
import './not-found.mjs';
3 changes: 2 additions & 1 deletion test/es-module/test-esm-require-cache.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import '../fixtures/es-module-require-cache/preload.js';
import '../fixtures/es-module-require-cache/counter.js';
import assert from 'assert';
Expand Down
3 changes: 2 additions & 1 deletion test/es-module/test-esm-shared-loader-dep.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-shared-dep.mjs
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import assert from 'assert';
import '../fixtures/es-modules/test-esm-ok.mjs';
import dep from '../fixtures/es-module-loaders/loader-dep.js';
Expand Down
3 changes: 2 additions & 1 deletion test/es-module/test-esm-shebang.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#! }]) // isn't js
// Flags: --experimental-modules
import '../common';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';

const isJs = true;
export default isJs;
7 changes: 4 additions & 3 deletions test/es-module/test-esm-snapshot.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Flags: --experimental-modules
import '../common';
import '../fixtures/es-modules/esm-snapshot-mutator';
import one from '../fixtures/es-modules/esm-snapshot';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import '../fixtures/es-modules/esm-snapshot-mutator.js';
import one from '../fixtures/es-modules/esm-snapshot.js';
import assert from 'assert';

assert.strictEqual(one, 1);
2 changes: 1 addition & 1 deletion test/es-module/test-esm-symlink-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const fs = require('fs');
tmpdir.refresh();

const realPath = path.resolve(__dirname, '../fixtures/es-modules/symlink.mjs');
const symlinkPath = path.resolve(tmpdir.path, 'symlink.js');
const symlinkPath = path.resolve(tmpdir.path, 'symlink.mjs');

try {
fs.symlinkSync(realPath, symlinkPath);
Expand Down
10 changes: 4 additions & 6 deletions test/es-module/test-esm-symlink.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const tmpDir = tmpdir.path;

const entry = path.join(tmpDir, 'entry.mjs');
const real = path.join(tmpDir, 'index.mjs');
const link_absolute_path = path.join(tmpDir, 'absolute');
const link_relative_path = path.join(tmpDir, 'relative');
const link_absolute_path = path.join(tmpDir, 'absolute.mjs');
const link_relative_path = path.join(tmpDir, 'relative.mjs');
const link_ignore_extension = path.join(tmpDir,
'ignore_extension.json');
const link_directory = path.join(tmpDir, 'directory');
Expand All @@ -22,15 +22,13 @@ fs.writeFileSync(real, 'export default [];');
fs.writeFileSync(entry, `
import assert from 'assert';
import real from './index.mjs';
import absolute from './absolute';
import relative from './relative';
import absolute from './absolute.mjs';
import relative from './relative.mjs';
import ignoreExtension from './ignore_extension.json';
import directory from './directory';

assert.strictEqual(absolute, real);
assert.strictEqual(relative, real);
assert.strictEqual(ignoreExtension, real);
assert.strictEqual(directory, real);
`);

try {
Expand Down
4 changes: 2 additions & 2 deletions test/es-module/test-esm-throw-undefined.mjs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Flags: --experimental-modules
import '../common';
import '../common/index.mjs';
import assert from 'assert';

async function doTest() {
await assert.rejects(
async () => {
await import('../fixtures/es-module-loaders/throw-undefined');
await import('../fixtures/es-module-loaders/throw-undefined.mjs');
},
(e) => e === undefined
);
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/es-module-loaders/syntax-error-import.mjs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
import { foo, notfound } from './module-named-exports';
import { foo, notfound } from './module-named-exports.mjs';
2 changes: 1 addition & 1 deletion test/fixtures/es-modules/loop.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { message } from './message';
import { message } from './message.mjs';

var t = 1;
var k = 1;
Expand Down
1 change: 0 additions & 1 deletion test/fixtures/es-modules/pjson-main/main.js

This file was deleted.

3 changes: 0 additions & 3 deletions test/fixtures/es-modules/pjson-main/package.json

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict';

// Trivial test to assert we can load files with `%` in their pathname.
// Imported by `test-esm-double-encoding.mjs`.

export default 42;
6 changes: 3 additions & 3 deletions test/message/esm_display_syntax_error_import.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Flags: --experimental-modules
/* eslint-disable no-unused-vars */
import '../common';
/* eslint-disable no-unused-vars, node-core/required-modules */
import '../common/index.mjs';
import {
foo,
notfound
} from '../fixtures/es-module-loaders/module-named-exports';
} from '../fixtures/es-module-loaders/module-named-exports.mjs';
2 changes: 1 addition & 1 deletion test/message/esm_display_syntax_error_import.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
file:///*/test/message/esm_display_syntax_error_import.mjs:6
notfound
^^^^^^^^
SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports' does not provide an export named 'notfound'
SyntaxError: The requested module '../fixtures/es-module-loaders/module-named-exports.mjs' does not provide an export named 'notfound'
at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*)
5 changes: 3 additions & 2 deletions test/message/esm_display_syntax_error_import_module.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --experimental-modules
import '../common';
import '../fixtures/es-module-loaders/syntax-error-import';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import '../fixtures/es-module-loaders/syntax-error-import.mjs';
4 changes: 2 additions & 2 deletions test/message/esm_display_syntax_error_import_module.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(node:*) ExperimentalWarning: The ESM module loader is experimental.
file:///*/test/fixtures/es-module-loaders/syntax-error-import.mjs:1
import { foo, notfound } from './module-named-exports';
import { foo, notfound } from './module-named-exports.mjs';
^^^^^^^^
SyntaxError: The requested module './module-named-exports' does not provide an export named 'notfound'
SyntaxError: The requested module './module-named-exports.mjs' does not provide an export named 'notfound'
at ModuleJob._instantiate (internal/modules/esm/module_job.js:*:*)
5 changes: 3 additions & 2 deletions test/message/esm_display_syntax_error_module.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --experimental-modules
import '../common';
import '../fixtures/es-module-loaders/syntax-error';
/* eslint-disable node-core/required-modules */
import '../common/index.mjs';
import '../fixtures/es-module-loaders/syntax-error.mjs';
2 changes: 1 addition & 1 deletion test/parallel/test-module-main-extension-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ const { execFileSync } = require('child_process');
const node = process.argv[0];

execFileSync(node, ['--experimental-modules',
fixtures.path('es-modules', 'test-esm-ok')]);
fixtures.path('es-modules', 'test-esm-ok.mjs')]);
execFileSync(node, ['--experimental-modules',
fixtures.path('es-modules', 'noext')]);