From 243f90283c6ccfa5b912469e6ebfdcd985abd165 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 6 Jan 2019 01:52:16 +0100 Subject: [PATCH] worker: remove `--experimental-worker` flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/25361 Reviewed-By: Rich Trott Reviewed-By: Yuta Hiroto Reviewed-By: Shingo Inoue Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Tobias Nießen Reviewed-By: Masashi Hirano Reviewed-By: Weijia Wang Reviewed-By: Gireesh Punathil Reviewed-By: Michael Dawson --- lib/internal/bootstrap/loaders.js | 3 +-- lib/internal/bootstrap/node.js | 4 +--- lib/internal/modules/cjs/helpers.js | 9 +-------- src/node_config.cc | 3 --- src/node_options.cc | 5 +---- src/node_options.h | 1 - 6 files changed, 4 insertions(+), 21 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index af4928cc6163d1..00776029f926f3 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -255,8 +255,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 index 516ea86738a66b..12001476e83621 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -179,9 +179,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 index fcb536c3df3738..1a2a91c509b011 100644 --- 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 index 4ff62b9c114c38..384872cc0092b7 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -89,9 +89,6 @@ static void Initialize(Local 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 index 979b53e25a9119..f4639907e81ee5 100644 --- 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 index 87a674b0055c32..aae3743306d50c 100644 --- 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 = true; bool expose_internals = false; std::string http_parser = #ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT