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

module: Unflag JSON modules #41736

Merged
merged 5 commits into from
Jan 31, 2022
Merged
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
8 changes: 0 additions & 8 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,6 @@ added:

Enable experimental `import.meta.resolve()` support.

### `--experimental-json-modules`

<!-- YAML
added: v12.9.0
-->

Enable experimental JSON support for the ES Module loader.

### `--experimental-loader=module`

<!-- YAML
Expand Down
45 changes: 6 additions & 39 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,22 +477,6 @@ These CommonJS variables are not available in ES modules.
`__filename` and `__dirname` use cases can be replicated via
[`import.meta.url`][].

#### No JSON Module Loading

JSON imports are still experimental and only supported via the
`--experimental-json-modules` flag.

Local JSON files can be loaded relative to `import.meta.url` with `fs` directly:

<!-- eslint-skip -->

```js
import { readFile } from 'fs/promises';
const json = JSON.parse(await readFile(new URL('./dat.json', import.meta.url)));
```

Alternatively `module.createRequire()` can be used.

#### No Native Module Loading

Native modules are not currently supported with ES module imports.
Expand Down Expand Up @@ -530,35 +514,19 @@ separate cache.

> Stability: 1 - Experimental

Currently importing JSON modules are only supported in the `commonjs` mode
and are loaded using the CJS loader. [WHATWG JSON modules specification][] are
still being standardized, and are experimentally supported by including the
additional flag `--experimental-json-modules` when running Node.js.

When the `--experimental-json-modules` flag is included, both the
`commonjs` and `module` mode use the new experimental JSON
loader. The imported JSON only exposes a `default`. There is no
support for named exports. A cache entry is created in the CommonJS
cache to avoid duplication. The same object is returned in
CommonJS if the JSON module has already been imported from the
same path.

Assuming an `index.mjs` with
JSON files can be referenced by `import`:

```js
import packageConfig from './package.json' assert { type: 'json' };
```

The `--experimental-json-modules` flag is needed for the module
to work.

```bash
node index.mjs # fails
node --experimental-json-modules index.mjs # works
```

The `assert { type: 'json' }` syntax is mandatory; see [Import Assertions][].

The imported JSON only exposes a `default` export. There is no support for named
Copy link

Choose a reason for hiding this comment

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

I might want a mention that the JSON is exposed as a normal object - one that can be modified and every import uses the same cached reference.

Copy link
Member

Choose a reason for hiding this comment

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

That's already how require works with JSON, and how imports of JSON in the browser will work. It's fine to mention it, but I'd also hope it's implied.

exports. A cache entry is created in the CommonJS cache to avoid duplication.
The same object is returned in CommonJS if the JSON module has already been
imported from the same path.
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved

<i id="esm_experimental_wasm_modules"></i>

## Wasm modules
Expand Down Expand Up @@ -1452,7 +1420,6 @@ success!
[Node.js Module Resolution Algorithm]: #resolver-algorithm-specification
[Terminology]: #terminology
[URL]: https://url.spec.whatwg.org/
[WHATWG JSON modules specification]: https://html.spec.whatwg.org/#creating-a-json-module-script
[`"exports"`]: packages.md#exports
[`"type"`]: packages.md#type
[`--input-type`]: cli.md#--input-typetype
Expand Down
3 changes: 1 addition & 2 deletions doc/api/packages.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ There is the ECMAScript module loader:
`'./startup/index.js'`) must be fully specified.
* It does no extension searching. A file extension must be provided
when the specifier is a relative or absolute file URL.
* It can load JSON modules, but an import assertion is required (behind
`--experimental-json-modules` flag).
* It can load JSON modules, but an import assertion is required.
* It accepts only `.js`, `.mjs`, and `.cjs` extensions for JavaScript text
files.
* It can be used to load JavaScript CommonJS modules. Such modules
Expand Down
3 changes: 0 additions & 3 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,6 @@ Enable Source Map V3 support for stack traces.
.It Fl -experimental-import-meta-resolve
Enable experimental ES modules support for import.meta.resolve().
.
.It Fl -experimental-json-modules
Enable experimental JSON interop support for the ES Module loader.
.
.It Fl -experimental-loader Ns = Ns Ar module
Specify the
.Ar module
Expand Down
11 changes: 4 additions & 7 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const {
const { extname } = require('path');
const { getOptionValue } = require('internal/options');

const experimentalJsonModules = getOptionValue('--experimental-json-modules');
const experimentalSpecifierResolution =
getOptionValue('--experimental-specifier-resolution');
const experimentalWasmModules = getOptionValue('--experimental-wasm-modules');
Expand All @@ -20,7 +19,8 @@ const extensionFormatMap = {
'__proto__': null,
'.cjs': 'commonjs',
'.js': 'module',
'.mjs': 'module'
'.json': 'json',
'.mjs': 'module',
};

const legacyExtensionFormatMap = {
Expand All @@ -29,17 +29,14 @@ const legacyExtensionFormatMap = {
'.js': 'commonjs',
'.json': 'commonjs',
'.mjs': 'module',
'.node': 'commonjs'
'.node': 'commonjs',
};

let experimentalSpecifierResolutionWarned = false;

if (experimentalWasmModules)
extensionFormatMap['.wasm'] = legacyExtensionFormatMap['.wasm'] = 'wasm';

if (experimentalJsonModules)
extensionFormatMap['.json'] = legacyExtensionFormatMap['.json'] = 'json';

const protocolHandlers = ObjectAssign(ObjectCreate(null), {
'data:'(parsed) {
const { 1: mime } = RegExpPrototypeExec(
Expand All @@ -49,7 +46,7 @@ const protocolHandlers = ObjectAssign(ObjectCreate(null), {
const format = ({
'__proto__': null,
'text/javascript': 'module',
'application/json': experimentalJsonModules ? 'json' : null,
'application/json': 'json',
'application/wasm': experimentalWasmModules ? 'wasm' : null
})[mime] || null;

Expand Down
10 changes: 2 additions & 8 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -315,19 +315,13 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
kAllowedInEnvironment);
AddOption("--experimental-abortcontroller", "",
NoOp{}, kAllowedInEnvironment);
AddOption("--experimental-json-modules",
"experimental JSON interop support for the ES Module loader",
&EnvironmentOptions::experimental_json_modules,
kAllowedInEnvironment);
AddOption("--experimental-json-modules", "", NoOp{}, kAllowedInEnvironment);
AddOption("--experimental-loader",
"use the specified module as a custom loader",
&EnvironmentOptions::userland_loader,
kAllowedInEnvironment);
AddAlias("--loader", "--experimental-loader");
AddOption("--experimental-modules",
"",
&EnvironmentOptions::experimental_modules,
kAllowedInEnvironment);
AddOption("--experimental-modules", "", NoOp{}, kAllowedInEnvironment);
AddOption("--experimental-wasm-modules",
"experimental ES Module support for webassembly modules",
&EnvironmentOptions::experimental_wasm_modules,
Expand Down
2 changes: 0 additions & 2 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,6 @@ class EnvironmentOptions : public Options {
std::vector<std::string> conditions;
std::string dns_result_order;
bool enable_source_maps = false;
bool experimental_json_modules = false;
bool experimental_modules = false;
std::string experimental_specifier_resolution;
bool experimental_wasm_modules = false;
bool experimental_import_meta_resolve = false;
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-assertionless-json-import.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Flags: --experimental-json-modules --experimental-loader ./test/fixtures/es-module-loaders/assertionless-json-import.mjs
// Flags: --experimental-loader ./test/fixtures/es-module-loaders/assertionless-json-import.mjs
'use strict';
const common = require('../common');
const { strictEqual } = require('assert');
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-data-urls.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
'use strict';
const common = require('../common');
const assert = require('assert');
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-dynamic-import-assertion.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
'use strict';
const common = require('../common');
const { strictEqual } = require('assert');
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-dynamic-import-assertion.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';
import { strictEqual } from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-import-assertion-1.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';
import { strictEqual } from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-import-assertion-2.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';
import { strictEqual } from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-import-assertion-3.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';
import { strictEqual } from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-import-assertion-4.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';
import { strictEqual } from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-import-assertion-errors.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
'use strict';
const common = require('../common');
const { rejects } = require('assert');
Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-import-assertion-errors.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';
import { rejects } from 'assert';

Expand Down
1 change: 0 additions & 1 deletion test/es-module/test-esm-import-json-named-export.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { spawn } from 'child_process';
import { execPath } from 'process';

const child = spawn(execPath, [
'--experimental-json-modules',
path('es-modules', 'import-json-named-export.mjs'),
]);

Expand Down
14 changes: 4 additions & 10 deletions test/es-module/test-esm-invalid-data-urls.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,14 @@ const assert = require('assert');
(async () => {
await assert.rejects(import('data:text/plain,export default0'), {
code: 'ERR_INVALID_MODULE_SPECIFIER',
message:
'Invalid module "data:text/plain,export default0" has an unsupported ' +
'MIME type "text/plain"',
message: 'Invalid module "data:text/plain,export default0" has an unsupported MIME type "text/plain"',
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
});
await assert.rejects(import('data:text/plain;base64,'), {
code: 'ERR_INVALID_MODULE_SPECIFIER',
message:
'Invalid module "data:text/plain;base64," has an unsupported ' +
'MIME type "text/plain"',
message: 'Invalid module "data:text/plain;base64," has an unsupported MIME type "text/plain"',
});
await assert.rejects(import('data:application/json,[]'), {
await assert.rejects(import('data:text/css,.error { color: red; }'), {
code: 'ERR_INVALID_MODULE_SPECIFIER',
message:
'Invalid module "data:application/json,[]" has an unsupported ' +
'MIME type "application/json"',
message: 'Invalid module "data:text/css,.error { color: red; }" has an unsupported MIME type "text/css"',
});
})().then(common.mustCall());
1 change: 0 additions & 1 deletion test/es-module/test-esm-json-cache.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';

import { strictEqual, deepStrictEqual } from 'assert';
Expand Down
2 changes: 0 additions & 2 deletions test/es-module/test-esm-json.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-json-modules
import '../common/index.mjs';
import { path } from '../common/fixtures.mjs';
import { strictEqual, ok } from 'assert';
Expand All @@ -10,7 +9,6 @@ strictEqual(secret.ofLife, 42);

// Test warning message
const child = spawn(process.execPath, [
'--experimental-json-modules',
path('/es-modules/json-modules.mjs'),
]);

Expand Down
21 changes: 0 additions & 21 deletions test/es-module/test-esm-non-js.js

This file was deleted.

23 changes: 23 additions & 0 deletions test/es-module/test-esm-non-js.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { mustCall } from '../common/index.mjs';
import { fileURL } from '../common/fixtures.mjs';
import { match, strictEqual } from 'assert';
import { spawn } from 'child_process';
import { execPath } from 'process';

// Verify non-js extensions fail for ESM
const child = spawn(execPath, [
'--input-type=module',
'--eval',
`import ${JSON.stringify(fileURL('es-modules', 'file.unknown'))}`,
]);

let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', mustCall((code, signal) => {
strictEqual(code, 1);
strictEqual(signal, null);
match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/);
}));