worker: remove --experimental-worker flag

addaleax · addaleax · commit 63d4cae009e1 · 2019-01-09T00:31:33.000+01:00
Having an experimental feature behind a flag makes change if we are expecting significant breaking changes to its API. Since the Worker API has been essentially stable since its initial introduction, and no noticeable doubt about possibly not keeping the feature around has been voiced, removing the flag and thereby reducing the barrier to experimentation, and consequently receiving feedback on the implementation, seems like a good idea. PR-URL: nodejs#25361 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
diff --git a/benchmark/misc/startup.js b/benchmark/misc/startup.js
@@ -10,7 +10,7 @@ const bench = common.createBenchmark(main, {
   script: ['benchmark/fixtures/require-cachable', 'test/fixtures/semicolon'],
   mode: ['process', 'worker']
 }, {
-  flags: ['--expose-internals', '--experimental-worker']  // for workers
+  flags: ['--expose-internals']
 });
 
 function spawnProcess(script) {
diff --git a/benchmark/worker/echo.js b/benchmark/worker/echo.js
@@ -7,7 +7,7 @@ const bench = common.createBenchmark(main, {
   payload: ['string', 'object'],
   sendsPerBroadcast: [1, 10],
   n: [1e5]
-}, { flags: ['--experimental-worker'] });
+});
 
 const workerPath = path.resolve(__dirname, '..', 'fixtures', 'echo.worker.js');
 
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -104,13 +104,6 @@ added: v9.6.0
 
 Enable experimental ES Module support in the `vm` module.
 
-### `--experimental-worker`
-<!-- YAML
-added: v10.5.0
--->
-
-Enable experimental worker threads using the `worker_threads` module.
-
 ### `--force-fips`
 <!-- YAML
 added: v6.0.0
@@ -620,7 +613,6 @@ Node.js options that are allowed are:
 - `--experimental-modules`
 - `--experimental-repl-await`
 - `--experimental-vm-modules`
-- `--experimental-worker`
 - `--force-fips`
 - `--icu-data-dir`
 - `--inspect`
diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md
@@ -6,7 +6,7 @@
 
 The `worker` module provides a way to create multiple environments running
 on independent threads, and to create message channels between them. It
-can be accessed using the `--experimental-worker` flag and:
+can be accessed using:
 
 ```js
 const worker = require('worker_threads');
diff --git a/doc/node.1 b/doc/node.1
@@ -94,9 +94,6 @@ keyword support in REPL.
 .It Fl -experimental-vm-modules
 Enable experimental ES module support in VM module.
 .
-.It Fl -experimental-worker
-Enable experimental worker threads using worker_threads module.
-.
 .It Fl -force-fips
 Force FIPS-compliant crypto on startup
 (Cannot be disabled from script code).
diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js
@@ -253,8 +253,7 @@ if (config.exposeInternals) {
   };
 
   NativeModule.isInternal = function(id) {
-    return id.startsWith('internal/') ||
-        (id === 'worker_threads' && !config.experimentalWorker);
+    return id.startsWith('internal/');
   };
 }
 
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -187,9 +187,7 @@ function startup() {
     setupQueueMicrotask();
   }
 
-  if (getOptionValue('--experimental-worker')) {
-    setupDOMException();
-  }
+  setupDOMException();
 
   // On OpenBSD process.execPath will be relative unless we
   // get the full path before process.execPath is used.
diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js
@@ -9,8 +9,6 @@ const {
   CHAR_HASH,
 } = require('internal/constants');
 
-const { getOptionValue } = require('internal/options');
-
 // Invoke with makeRequireFunction(module) where |module| is the Module object
 // to use as the context for the require() function.
 function makeRequireFunction(mod) {
@@ -100,14 +98,9 @@ const builtinLibs = [
   'dgram', 'dns', 'domain', 'events', 'fs', 'http', 'http2', 'https', 'net',
   'os', 'path', 'perf_hooks', 'punycode', 'querystring', 'readline', 'repl',
   'stream', 'string_decoder', 'tls', 'trace_events', 'tty', 'url', 'util',
-  'v8', 'vm', 'zlib'
+  'v8', 'vm', 'worker_threads', 'zlib'
 ];
 
-if (getOptionValue('--experimental-worker')) {
-  builtinLibs.push('worker_threads');
-  builtinLibs.sort();
-}
-
 if (typeof internalBinding('inspector').open === 'function') {
   builtinLibs.push('inspector');
   builtinLibs.sort();
diff --git a/src/node_config.cc b/src/node_config.cc
@@ -85,9 +85,6 @@ static void Initialize(Local<Object> target,
   if (env->options()->experimental_vm_modules)
     READONLY_TRUE_PROPERTY(target, "experimentalVMModules");
 
-  if (env->options()->experimental_worker)
-    READONLY_TRUE_PROPERTY(target, "experimentalWorker");
-
   if (env->options()->experimental_repl_await)
     READONLY_TRUE_PROPERTY(target, "experimentalREPLAwait");
 
diff --git a/src/node_options.cc b/src/node_options.cc
@@ -109,10 +109,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "experimental ES Module support in vm module",
             &EnvironmentOptions::experimental_vm_modules,
             kAllowedInEnvironment);
-  AddOption("--experimental-worker",
-            "experimental threaded Worker support",
-            &EnvironmentOptions::experimental_worker,
-            kAllowedInEnvironment);
+  AddOption("--experimental-worker", "", NoOp{}, kAllowedInEnvironment);
   AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
   AddOption("--http-parser",
             "Select which HTTP parser to use; either 'legacy' or 'llhttp' "
diff --git a/src/node_options.h b/src/node_options.h
@@ -96,7 +96,6 @@ class EnvironmentOptions : public Options {
   bool experimental_modules = false;
   bool experimental_repl_await = false;
   bool experimental_vm_modules = false;
-  bool experimental_worker = false;
   bool expose_internals = false;
   std::string http_parser = "llhttp";
   bool no_deprecation = false;
diff --git a/test/abort/test-addon-uv-handle-leak.js b/test/abort/test-addon-uv-handle-leak.js
@@ -36,8 +36,7 @@ if (process.argv[2] === 'child') {
   binding.leakHandle(0x42);
   `, { eval: true });
 } else {
-  const child = cp.spawnSync(process.execPath,
-                             ['--experimental-worker', __filename, 'child']);
+  const child = cp.spawnSync(process.execPath, [__filename, 'child']);
   const stderr = child.stderr.toString();
 
   assert.strictEqual(child.stdout.toString(), '');
diff --git a/test/addons/hello-world/test-worker.js b/test/addons/hello-world/test-worker.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../../common');
 const assert = require('assert');
diff --git a/test/addons/worker-addon/test.js b/test/addons/worker-addon/test.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../../common');
 const assert = require('assert');
@@ -11,7 +10,6 @@ if (process.argv[2] === 'child') {
   new Worker(`require(${JSON.stringify(binding)});`, { eval: true });
 } else {
   const proc = child_process.spawnSync(process.execPath, [
-    '--experimental-worker',
     __filename,
     'child'
   ]);
diff --git a/test/parallel/test-async-wrap-missing-method.js b/test/parallel/test-async-wrap-missing-method.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-heapdump-worker.js b/test/parallel/test-heapdump-worker.js
@@ -1,4 +1,4 @@
-// Flags: --expose-internals --experimental-worker
+// Flags: --expose-internals
 'use strict';
 require('../common');
 const { validateSnapshotNodes } = require('../common/heap');
diff --git a/test/parallel/test-module-cjs-helpers.js b/test/parallel/test-module-cjs-helpers.js
@@ -1,5 +1,5 @@
 'use strict';
-// Flags: --expose-internals --experimental-worker
+// Flags: --expose-internals
 
 require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-trace-events-api-worker-disabled.js b/test/parallel/test-trace-events-api-worker-disabled.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-trace-events-dynamic-enable-workers-disabled.js b/test/parallel/test-trace-events-dynamic-enable-workers-disabled.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-trace-events-worker-metadata.js b/test/parallel/test-trace-events-worker-metadata.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
@@ -15,8 +14,7 @@ if (isMainThread) {
   process.chdir(tmpdir.path);
 
   const proc = cp.spawn(process.execPath,
-                        [ '--experimental-worker',
-                          '--trace-event-categories', 'node',
+                        [ '--trace-event-categories', 'node',
                           '-e', CODE ]);
   proc.once('exit', common.mustCall(() => {
     assert(fs.existsSync(FILE_NAME));
diff --git a/test/parallel/test-v8-coverage.js b/test/parallel/test-v8-coverage.js
@@ -87,7 +87,6 @@ function nextdir() {
 {
   const coverageDirectory = path.join(tmpdir.path, nextdir());
   const output = spawnSync(process.execPath, [
-    '--experimental-worker',
     require.resolve('../fixtures/v8-coverage/worker')
   ], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
   assert.strictEqual(output.status, 0);
diff --git a/test/parallel/test-worker-cleanup-handles.js b/test/parallel/test-worker-cleanup-handles.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-debug.js b/test/parallel/test-worker-debug.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 
@@ -154,9 +153,9 @@ async function testBasicWorkerDebug(session, post) {
   await workerSession.post('Debugger.enable');
   await workerSession.post('Runtime.enable');
   await workerSession.waitForBreakAfterCommand(
-    'Runtime.runIfWaitingForDebugger', __filename, 2);
+    'Runtime.runIfWaitingForDebugger', __filename, 1);
   await workerSession.waitForBreakAfterCommand(
-    'Debugger.resume', __filename, 27);  // V8 line number is zero-based
+    'Debugger.resume', __filename, 26);  // V8 line number is zero-based
   assert.strictEqual(await consolePromise, workerMessage);
   workerSession.post('Debugger.resume');
   await Promise.all([worker, detached, contextEvents]);
diff --git a/test/parallel/test-worker-dns-terminate.js b/test/parallel/test-worker-dns-terminate.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const { Worker } = require('worker_threads');
diff --git a/test/parallel/test-worker-esmodule.js b/test/parallel/test-worker-esmodule.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-worker --experimental-modules
+// Flags: --experimental-modules
 'use strict';
 const common = require('../common');
 const fixtures = require('../common/fixtures');
diff --git a/test/parallel/test-worker-exit-code.js b/test/parallel/test-worker-exit-code.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 
diff --git a/test/parallel/test-worker-invalid-workerdata.js b/test/parallel/test-worker-invalid-workerdata.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-memory.js b/test/parallel/test-worker-memory.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-message-channel-sharedarraybuffer.js b/test/parallel/test-worker-message-channel-sharedarraybuffer.js
@@ -1,4 +1,4 @@
-// Flags: --expose-gc --experimental-worker
+// Flags: --expose-gc
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-worker-message-channel.js b/test/parallel/test-worker-message-channel.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-message-port-arraybuffer.js b/test/parallel/test-worker-message-port-arraybuffer.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-message-port-drain.js b/test/parallel/test-worker-message-port-drain.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 require('../common');
 
diff --git a/test/parallel/test-worker-message-port-message-port-transferring.js b/test/parallel/test-worker-message-port-message-port-transferring.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-message-port-transfer-closed.js b/test/parallel/test-worker-message-port-transfer-closed.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-worker-message-port-transfer-self.js b/test/parallel/test-worker-message-port-transfer-self.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-worker-message-port-transfer-target.js b/test/parallel/test-worker-message-port-transfer-target.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-worker-message-port-wasm-module.js b/test/parallel/test-worker-message-port-wasm-module.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-message-port-wasm-threads.js b/test/parallel/test-worker-message-port-wasm-threads.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-worker --experimental-wasm-threads
+// Flags: --experimental-wasm-threads
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-message-port.js b/test/parallel/test-worker-message-port.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-messageport-transfer-terminate.js b/test/parallel/test-worker-messageport-transfer-terminate.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 require('../common');
 const { Worker, MessageChannel } = require('worker_threads');
diff --git a/test/parallel/test-worker-nexttick-terminate.js b/test/parallel/test-worker-nexttick-terminate.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const { Worker } = require('worker_threads');
diff --git a/test/parallel/test-worker-onmessage-not-a-function.js b/test/parallel/test-worker-onmessage-not-a-function.js
@@ -3,7 +3,6 @@
 // listener from holding the event loop open. This test confirms that
 // functionality.
 
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const { Worker, parentPort } = require('worker_threads');
diff --git a/test/parallel/test-worker-onmessage.js b/test/parallel/test-worker-onmessage.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-parent-port-ref.js b/test/parallel/test-worker-parent-port-ref.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const assert = require('assert');
 const common = require('../common');
diff --git a/test/parallel/test-worker-relative-path-double-dot.js b/test/parallel/test-worker-relative-path-double-dot.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const path = require('path');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-relative-path.js b/test/parallel/test-worker-relative-path.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const path = require('path');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-stdio.js b/test/parallel/test-worker-stdio.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-syntax-error-file.js b/test/parallel/test-worker-syntax-error-file.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const fixtures = require('../common/fixtures');
diff --git a/test/parallel/test-worker-syntax-error.js b/test/parallel/test-worker-syntax-error.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-type-check.js b/test/parallel/test-worker-type-check.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 
 const common = require('../common');
diff --git a/test/parallel/test-worker-uncaught-exception-async.js b/test/parallel/test-worker-uncaught-exception-async.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-uncaught-exception.js b/test/parallel/test-worker-uncaught-exception.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-unsupported-path.js b/test/parallel/test-worker-unsupported-path.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 
 const path = require('path');
diff --git a/test/parallel/test-worker-unsupported-things.js b/test/parallel/test-worker-unsupported-things.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/parallel/test-worker-workerdata-sharedarraybuffer.js b/test/parallel/test-worker-workerdata-sharedarraybuffer.js
diff --git a/test/parallel/test-worker.js b/test/parallel/test-worker.js
diff --git a/test/wpt/test-console.js b/test/wpt/test-console.js
diff --git a/test/wpt/test-url.js b/test/wpt/test-url.js
diff --git a/tools/test.py b/tools/test.py

Original file line number	Diff line number	Diff line change
`@@ -253,8 +253,7 @@ if (config.exposeInternals) {`
`253`	`253`	`};`
`254`	`254`
`255`	`255`	`NativeModule.isInternal = function(id) {`
`256`		`- return id.startsWith('internal/') \|\|`
`257`		`- (id === 'worker_threads' && !config.experimentalWorker);`
	`256`	`+ return id.startsWith('internal/');`
`258`	`257`	`};`
`259`	`258`	`}`
`260`	`259`
Original file line number	Diff line number	Diff line change
`@@ -187,9 +187,7 @@ function startup() {`
`187`	`187`	`setupQueueMicrotask();`
`188`	`188`	`}`
`189`	`189`
`190`		`- if (getOptionValue('--experimental-worker')) {`
`191`		`- setupDOMException();`
`192`		`- }`
	`190`	`+ setupDOMException();`
`193`	`191`
`194`	`192`	`// On OpenBSD process.execPath will be relative unless we`
`195`	`193`	`// get the full path before process.execPath is used.`