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.

Sign up for GitHub

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: Revert remove experimental status from JSON modules #29754

Closed
wants to merge 4 commits into from
Closed

module: Revert remove experimental status from JSON modules #29754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b033f83
Revert "esm: remove experimental status from JSON modules"
guybedford Sep 28, 2019
5dc4167
fixup broken option
guybedford Oct 1, 2019
67d6171
add to NODE_OPTIONS list
guybedford Oct 1, 2019
7675c12
fixup doc spec link
guybedford Oct 2, 2019
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: 8 additions & 0 deletions doc/api/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@ added: v12.7.0

Enable experimental resolution using the `exports` field in `package.json`.

### `--experimental-json-modules`
<!-- YAML
added: v12.9.0
-->

Enable experimental JSON support for the ES Module loader.

### `--experimental-modules`
<!-- YAML
added: v8.5.0
Expand Down Expand Up @@ -982,6 +989,7 @@ Node.js options that are allowed are:
* `--enable-fips`
* `--es-module-specifier-resolution`
* `--experimental-exports`
* `--experimental-json-modules`
* `--experimental-loader`
* `--experimental-modules`
* `--experimental-policy`
Expand Down
19 changes: 16 additions & 3 deletions doc/api/esm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,11 +584,16 @@ fs.readFileSync = () => Buffer.from('Hello, ESM');
fs.readFileSync === readFileSync;
```

## JSON Modules
## Experimental JSON Modules

JSON modules follow the [WHATWG JSON modules specification][].
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.

The imported JSON only exposes a `default`. There is no
When the `--experimental-json-modules` flag is included both the
`commonjs` and `module` mode will 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 will be returned in
CommonJS if the JSON module has already been imported from the
Expand All @@ -601,6 +606,14 @@ Assuming an `index.mjs` with
import packageConfig from './package.json';
```

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

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

## Experimental Wasm Modules

Importing Web Assembly modules is supported under the
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ Requires Node.js to be built with
.It Fl -es-module-specifier-resolution
Select extension resolution algorithm for ES Modules; either 'explicit' (default) or 'node'
.
.It Fl -experimental-json-modules
Enable experimental JSON interop support for the ES Module loader.
.
.It Fl -experimental-modules
Enable experimental ES module support and caching modules.
.
Expand Down
7 changes: 5 additions & 2 deletions lib/internal/modules/esm/default_resolve.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const { getOptionValue } = require('internal/options');

const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const experimentalJsonModules = getOptionValue('--experimental-json-modules');
const typeFlag = getOptionValue('--input-type');
const experimentalWasmModules = getOptionValue('--experimental-wasm-modules');
const { resolve: moduleWrapResolve,
Expand All @@ -28,22 +29,24 @@ const extensionFormatMap = {
'__proto__': null,
'.cjs': 'commonjs',
'.js': 'module',
'.json': 'json',
'.mjs': 'module'
};

const legacyExtensionFormatMap = {
'__proto__': null,
'.cjs': 'commonjs',
'.js': 'commonjs',
'.json': 'json',
'.json': 'commonjs',
'.mjs': 'module',
'.node': 'commonjs'
};

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

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

function resolve(specifier, parentURL) {
try {
const parsed = new URL(specifier);
Expand Down
9 changes: 9 additions & 0 deletions src/node_options.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
}
}

if (experimental_json_modules && !experimental_modules) {
errors->push_back("--experimental-json-modules requires "
"--experimental-modules be enabled");
}

if (experimental_wasm_modules && !experimental_modules) {
errors->push_back("--experimental-wasm-modules requires "
"--experimental-modules be enabled");
Expand Down Expand Up @@ -312,6 +317,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"experimental support for exports in package.json",
&EnvironmentOptions::experimental_exports,
Copy link
Member

@Trott Trott Oct 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It appears that this isn't valid C++ and will not compile.

EDIT: Well, this line is the start of a valid statement, but then the next few lines....you probably get what I mean. This:

20:46:37 ../src/node_options.cc: In constructor ‘node::options_parser::EnvironmentOptionsParser::EnvironmentOptionsParser()’:
20:46:37 ../src/node_options.cc:321:35: error: expected ‘)’ before ‘;’ token
20:46:37              kAllowedInEnvironment);
20:46:37

kAllowedInEnvironment);
AddOption("--experimental-json-modules",
"experimental JSON interop support for the ES Module loader",
&EnvironmentOptions::experimental_json_modules,
kAllowedInEnvironment);
AddOption("--experimental-loader",
"(with --experimental-modules) use the specified file as a "
"custom loader",
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class EnvironmentOptions : public Options {
public:
bool abort_on_uncaught_exception = false;
bool experimental_exports = false;
bool experimental_json_modules = false;
bool experimental_modules = false;
std::string es_module_specifier_resolution;
bool experimental_wasm_modules = false;
Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-json-cache.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Flags: --experimental-modules
// Flags: --experimental-modules --experimental-json-modules
import '../common/index.mjs';

import { strictEqual, deepStrictEqual } from 'assert';
Expand Down
3 changes: 1 addition & 2 deletions test/es-module/test-esm-json.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Flags: --experimental-modules

// Flags: --experimental-modules --experimental-json-modules
import '../common/index.mjs';
import { strictEqual } from 'assert';

Expand Down