esm: Remove --loader.

PR-URL: #6 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
nodejs · Feb 16, 2019 · ed6814e · ed6814e
1 parent 35461ad
commit ed6814e
Show file tree

Hide file tree

Showing 41 changed files with 2 additions and 467 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -39,7 +39,6 @@ module.exports = {
       files: [
         'doc/api/esm.md',
         '*.mjs',
-        'test/es-module/test-esm-example-loader.js',
       ],
       parserOptions: { sourceType: 'module' },
     },

diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -256,13 +256,6 @@ default) is not firewall-protected.**
 
 See the [debugging security implications][] section for more information.
 
-### `--loader=file`
-<!-- YAML
-added: v9.0.0
--->
-
-Specify the `file` of the custom [experimental ECMAScript Module][] loader.
-
 ### `--max-http-header-size=size`
 <!-- YAML
 added: v11.6.0
@@ -699,7 +692,6 @@ Node.js options that are allowed are:
 - `--inspect`
 - `--inspect-brk`
 - `--inspect-port`
-- `--loader`
 - `--max-http-header-size`
 - `--napi-modules`
 - `--no-deprecation`
@@ -887,7 +879,6 @@ greater than `4` (its current default value). For more information, see the
 [debugger]: debugger.html
 [debugging security implications]: https://nodejs.org/en/docs/guides/debugging-getting-started/#security-implications
 [emit_warning]: process.html#process_process_emitwarning_warning_type_code_ctor
-[experimental ECMAScript Module]: esm.html#esm_loader_hooks
 [libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html
 [remote code execution]: https://www.owasp.org/index.php/Code_Injection
 [secureProtocol]: tls.html#tls_tls_createsecurecontext_options
diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -146,125 +146,6 @@ fs.readFileSync = () => Buffer.from('Hello, ESM');
 fs.readFileSync === readFileSync;
 ```
 
-## Loader hooks
-
-<!-- type=misc -->
-
-To customize the default module resolution, loader hooks can optionally be
-provided via a `--loader ./loader-name.mjs` argument to Node.js.
-
-When hooks are used they only apply to ES module loading and not to any
-CommonJS modules loaded.
-
-### Resolve hook
-
-The resolve hook returns the resolved file URL and module format for a
-given module specifier and parent file URL:
-
-```js
-const baseURL = new URL('file://');
-baseURL.pathname = `${process.cwd()}/`;
-
-export async function resolve(specifier,
-                              parentModuleURL = baseURL,
-                              defaultResolver) {
-  return {
-    url: new URL(specifier, parentModuleURL).href,
-    format: 'esm'
-  };
-}
-```
-
-The `parentModuleURL` is provided as `undefined` when performing main Node.js
-load itself.
-
-The default Node.js ES module resolution function is provided as a third
-argument to the resolver for easy compatibility workflows.
-
-In addition to returning the resolved file URL value, the resolve hook also
-returns a `format` property specifying the module format of the resolved
-module. This can be one of the following:
-
-| `format` | Description |
-| --- | --- |
-| `'esm'` | Load a standard JavaScript module |
-| `'builtin'` | Load a node builtin CommonJS module |
-| `'dynamic'` | Use a [dynamic instantiate hook][] |
-
-For example, a dummy loader to load JavaScript restricted to browser resolution
-rules with only JS file extension and Node.js builtin modules support could
-be written:
-
-```js
-import path from 'path';
-import process from 'process';
-import Module from 'module';
-
-const builtins = Module.builtinModules;
-const JS_EXTENSIONS = new Set(['.js', '.mjs']);
-
-const baseURL = new URL('file://');
-baseURL.pathname = `${process.cwd()}/`;
-
-export function resolve(specifier, parentModuleURL = baseURL, defaultResolve) {
-  if (builtins.includes(specifier)) {
-    return {
-      url: specifier,
-      format: 'builtin'
-    };
-  }
-  if (/^\.{0,2}[/]/.test(specifier) !== true && !specifier.startsWith('file:')) {
-    // For node_modules support:
-    // return defaultResolve(specifier, parentModuleURL);
-    throw new Error(
-      `imports must begin with '/', './', or '../'; '${specifier}' does not`);
-  }
-  const resolved = new URL(specifier, parentModuleURL);
-  const ext = path.extname(resolved.pathname);
-  if (!JS_EXTENSIONS.has(ext)) {
-    throw new Error(
-      `Cannot load file with non-JavaScript file extension ${ext}.`);
-  }
-  return {
-    url: resolved.href,
-    format: 'esm'
-  };
-}
-```
-
-With this loader, running:
-
-```console
-NODE_OPTIONS='--experimental-modules --loader ./custom-loader.mjs' node x.js
-```
-
-would load the module `x.js` as an ES module with relative resolution support
-(with `node_modules` loading skipped in this example).
-
-### Dynamic instantiate hook
-
-To create a custom dynamic module that doesn't correspond to one of the
-existing `format` interpretations, the `dynamicInstantiate` hook can be used.
-This hook is called only for modules that return `format: 'dynamic'` from
-the `resolve` hook.
-
-```js
-export async function dynamicInstantiate(url) {
-  return {
-    exports: ['customExportName'],
-    execute: (exports) => {
-      // Get and set functions provided for pre-allocated export names
-      exports.customExportName.set('value');
-    }
-  };
-}
-```
-
-With the list of module exports provided upfront, the `execute` function will
-then be called at the exact point of module evaluation order for that module
-in the import tree.
-
 [Node.js EP for ES Modules]: https://github.com/nodejs/node-eps/blob/master/002-es-modules.md
-[dynamic instantiate hook]: #esm_dynamic_instantiate_hook
 [`module.createRequireFromPath()`]: modules.html#modules_module_createrequirefrompath_filename
 [ESM Minimal Kernel]: https://github.com/nodejs/modules/blob/master/doc/plan-for-new-modules-implementation.md
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -48,7 +48,6 @@ const experimentalModules = getOptionValue('--experimental-modules');
 const manifest = getOptionValue('--experimental-policy') ?
   require('internal/process/policy').manifest :
   null;
-const hasLoader = getOptionValue('--loader');
 
 const {
   ERR_INVALID_ARG_VALUE,
@@ -802,7 +801,7 @@ Module.runMain = function() {
   const ext = path.extname(base);
   const isESM = ext === '.mjs';
 
-  if (experimentalModules && (isESM || hasLoader)) {
+  if (experimentalModules && isESM) {
     if (asyncESM === undefined) lazyLoadESM();
     asyncESM.loaderPromise.then((loader) => {
       return loader.import(pathToFileURL(process.argv[1]).pathname);

diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js
@@ -4,7 +4,6 @@ const {
   callbackMap,
 } = internalBinding('module_wrap');
 
-const { pathToFileURL } = require('internal/url');
 const Loader = require('internal/modules/esm/loader');
 const {
   wrapToModuleMap,
@@ -41,15 +40,8 @@ exports.loaderPromise = new Promise((resolve, reject) => {
 exports.ESMLoader = undefined;
 
 exports.initializeLoader = function(cwd, userLoader) {
-  let ESMLoader = new Loader();
+  const ESMLoader = new Loader();
   const loaderPromise = (async () => {
-    if (userLoader) {
-      const hooks = await ESMLoader.import(
-        userLoader, pathToFileURL(`${cwd}/`).href);
-      ESMLoader = new Loader();
-      ESMLoader.hook(hooks);
-      exports.ESMLoader = ESMLoader;
-    }
     return ESMLoader;
   })();
   loaderResolve(loaderPromise);

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -99,10 +99,6 @@ void PerIsolateOptions::CheckOptions(std::vector<std::string>* errors) {
 }
 
 void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
-  if (!userland_loader.empty() && !experimental_modules) {
-    errors->push_back("--loader requires --experimental-modules be enabled");
-  }
-
   if (syntax_check_only && has_eval_string) {
     errors->push_back("either --check or --eval can be used, not both");
   }
@@ -199,11 +195,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "(default: llhttp).",
             &EnvironmentOptions::http_parser,
             kAllowedInEnvironment);
-  AddOption("--loader",
-            "(with --experimental-modules) use the specified file as a "
-            "custom loader",
-            &EnvironmentOptions::userland_loader,
-            kAllowedInEnvironment);
   AddOption("--no-deprecation",
             "silence deprecation warnings",
             &EnvironmentOptions::no_deprecation,

diff --git a/src/node_options.h b/src/node_options.h
@@ -103,7 +103,6 @@ class EnvironmentOptions : public Options {
   bool trace_deprecation = false;
   bool trace_sync_io = false;
   bool trace_warnings = false;
-  std::string userland_loader;
 
   bool syntax_check_only = false;
   bool has_eval_string = false;

diff --git a/test/es-module/test-esm-example-loader.js b/test/es-module/test-esm-example-loader.js
diff --git a/test/es-module/test-esm-loader-dependency.mjs b/test/es-module/test-esm-loader-dependency.mjs
diff --git a/test/es-module/test-esm-loader-invalid-format.mjs b/test/es-module/test-esm-loader-invalid-format.mjs
diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs
diff --git a/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs b/test/es-module/test-esm-loader-missing-dynamic-instantiate-hook.mjs
diff --git a/test/es-module/test-esm-named-exports.mjs b/test/es-module/test-esm-named-exports.mjs
diff --git a/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs b/test/es-module/test-esm-preserve-symlinks-not-found-plain.mjs
diff --git a/test/es-module/test-esm-preserve-symlinks-not-found.mjs b/test/es-module/test-esm-preserve-symlinks-not-found.mjs
diff --git a/test/es-module/test-esm-resolve-hook.mjs b/test/es-module/test-esm-resolve-hook.mjs
diff --git a/test/es-module/test-esm-shared-loader-dep.mjs b/test/es-module/test-esm-shared-loader-dep.mjs
diff --git a/test/es-module/test-esm-throw-undefined.mjs b/test/es-module/test-esm-throw-undefined.mjs
diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs