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 #37375

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 0 additions & 7 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,13 +222,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
added: v9.0.0
Expand Down
52 changes: 20 additions & 32 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
<!-- YAML
added: v8.5.0
changes:
- version: REPLACEME
pr-url: https://githubcom/nodejs/node/pull/00000
description: Loading JSON modules no longer requires a command-line flag.
- version:
- v15.3.0
pr-url: https://github.com/nodejs/node/pull/35781
Expand Down Expand Up @@ -422,21 +425,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)));
```

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

#### No Native Module Loading

Native modules are not currently supported with ES module imports.
Expand Down Expand Up @@ -471,35 +459,35 @@ separate cache.
<i id="esm_experimental_json_modules"></i>

## JSON modules
<!--YAML
added: v12.9.0
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
changes:
- version: REPLACEME
pr-url: https://githubcom/nodejs/node/pull/00000
description: Loading JSON modules no longer requires a command-line flag.
-->

> 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
The imported JSON only exposes a `default` export. 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

<!-- eslint-skip -->
```js
```js esm
import packageConfig from './package.json';
const { version, name } = packageConfig;

console.log(version);
```

The `--experimental-json-modules` flag is needed for the module
to work.
```js cjs
(async () => {
const { default: { version, name } } = await import('./package.json');

```bash
node index.mjs # fails
node --experimental-json-modules index.mjs # works
console.log(version);
})().catch(console.error);
```

<i id="esm_experimental_wasm_modules"></i>
Expand Down
9 changes: 3 additions & 6 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,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 @@ -18,7 +17,8 @@ const extensionFormatMap = {
'__proto__': null,
'.cjs': 'commonjs',
'.js': 'module',
'.mjs': 'module'
'.mjs': 'module',
'.json': 'json',
};

const legacyExtensionFormatMap = {
Expand All @@ -33,9 +33,6 @@ const legacyExtensionFormatMap = {
if (experimentalWasmModules)
extensionFormatMap['.wasm'] = legacyExtensionFormatMap['.wasm'] = 'wasm';

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

function defaultGetFormat(url, context, defaultGetFormatUnused) {
if (StringPrototypeStartsWith(url, 'node:')) {
return { format: 'builtin' };
Expand All @@ -49,7 +46,7 @@ function defaultGetFormat(url, context, defaultGetFormatUnused) {
const format = ({
'__proto__': null,
'text/javascript': 'module',
'application/json': experimentalJsonModules ? 'json' : null,
'application/json': 'json',
'application/wasm': experimentalWasmModules ? 'wasm' : null
})[mime] || null;
return { format };
Expand Down
3 changes: 1 addition & 2 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
kAllowedInEnvironment);
AddOption("--experimental-abortcontroller", "",
NoOp{}, kAllowedInEnvironment);
AddOption("--experimental-json-modules",
"experimental JSON interop support for the ES Module loader",
AddOption("--experimental-json-modules", "",
&EnvironmentOptions::experimental_json_modules,
Copy link
Member

Choose a reason for hiding this comment

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

I think the environment option can be removed as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not on Node.js v15.x, v14.x, and v12.x. It can be removed in v16.x though.

kAllowedInEnvironment);
AddOption("--experimental-loader",
Expand Down
26 changes: 10 additions & 16 deletions test/es-module/test-esm-non-js.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
'use strict';

const common = require('../common');
const { spawn } = require('child_process');
const assert = require('assert');

const entry = require.resolve('./test-esm-json.mjs');
const fixtures = require('../common/fixtures');

// Verify non-js extensions fail for ESM
const child = spawn(process.execPath, [entry]);

let stderr = '';
child.stderr.setEncoding('utf8');
child.stderr.on('data', (data) => {
stderr += data;
});
child.on('close', common.mustCall((code, signal) => {
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1);
const assert = require('assert');
const { pathToFileURL } = require('url');
const { Worker } = require('worker_threads');

new Worker(new URL(
'data:text/javascript,' +
`import ${JSON.stringify(pathToFileURL(fixtures.path('altdocs.md')))};`
)).on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ERR_UNKNOWN_FILE_EXTENSION');
}));