worker: remove `--experimental-worker` flag #25361

addaleax · 2019-01-06T01:05:42Z

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.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

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.

nodejs-github-bot · 2019-01-06T01:05:43Z

@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2149/pipeline

addaleax · 2019-01-06T01:14:34Z

CI: https://ci.nodejs.org/job/node-test-pull-request/19948/

/cc @nodejs/workers

benchmark/misc/startup.js

hiroppy

😆

addaleax · 2019-01-06T17:03:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/19956/

gireeshpunathil · 2019-01-07T06:05:15Z

@addaleax - I was under the impression that the module name worker_threads is a place holder that can converge and freeze into something else before exiting experimental. Isn't that so?

addaleax · 2019-01-08T00:01:34Z

@gireeshpunathil If you want to pick another name, we can take up that conversation again – I think it’s not too late if we really want to changes this, but I’m not sure it’s worth it.

worker_threads is accurate and not too explicit as a name, imo. It wasn’t my first choice, but I’d rather leave it as it is than start the whole discussion again (and break the existing code that is out there, including guides/tutorials, even if it’s currently still a flagged feature).

gireeshpunathil · 2019-01-08T02:05:12Z

@addaleax - thanks, I agree. I was just making sure my understanding around the naming is correct, and wanted to confirm it as this is the last opportunity to talk about that, if at all. Agree that worker_threads is an appropriate name within the constraints (being meaningful and not overlapping with used names).

mhdawson

LGTM, assuming the feature is still documented as experimental.

addaleax · 2019-01-08T23:44:12Z

Landed in 63d4cae

Thanks for the reviews, everyone!

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>

This is similar to nodejs#25361 in functionality, but allows avoiding some backporting pain for v11.x. Refs: nodejs#25361

This is a trimmed-down version of 63d4cae that avoids backporting pain for v11.x. The remainder of the original commit can be cherry-picked later, once other PRs have been backported first. --- 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. Refs: 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>

BridgeAR · 2019-01-09T23:56:01Z

This is functionally backported to v11.x but some parts are not yet backported which await other commits to be backported first.

This is similar to #25361 in functionality, but allows avoiding some backporting pain for v11.x. PR-URL: #25404 Refs: #25361 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

This is a trimmed-down version of 63d4cae that avoids backporting pain for v11.x. The remainder of the original commit can be cherry-picked later, once other PRs have been backported first. PR-URL: #25404 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> --- Original commit message --- 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. Refs: #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>

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

This is similar to nodejs#25361 in functionality, but allows avoiding some backporting pain for v11.x. PR-URL: nodejs#25404 Refs: nodejs#25361 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

This is a trimmed-down version of 63d4cae that avoids backporting pain for v11.x. The remainder of the original commit can be cherry-picked later, once other PRs have been backported first. PR-URL: nodejs#25404 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> --- Original commit message --- 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. Refs: 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>

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>

Notable Changes * compression / zlib: * Added brotli support (Anna Henningsen and Zach Vacura) nodejs#24938 * console: * Added `inspectOptions` option (Ruben Bridgewater) nodejs#24978 * crypto: * Always accept private keys as public keys (Tobias Nießen) nodejs#25217 * deps: * Upgrade npm to v6.5.0 (Jordan Harband) nodejs#25234 * fs: * Use internalBinding('fs') internally instead of process.binding('fs') (Masashi Hirano) nodejs#22478 * http(s): * Support overriding http\\s.globalAgent (Roy Sommer) nodejs#25170 * util: * Inspect ArrayBuffers contents closely (Ruben Bridgewater) nodejs#25006 * worker: * Expose workers by default and remove `--experimental-worker` flag (Anna Henningsen) nodejs#25361 PR-URL: nodejs#25537

Notable Changes * compression / zlib: * Added brotli support (Anna Henningsen and Zach Vacura) #24938 * console: * Added `inspectOptions` option (Ruben Bridgewater) #24978 * crypto: * Always accept private keys as public keys (Tobias Nießen) #25217 * deps: * Upgrade npm to v6.5.0 (Jordan Harband) #25234 * fs: * Use internalBinding('fs') internally instead of process.binding('fs') (Masashi Hirano) #22478 * http(s): * Support overriding http\\s.globalAgent (Roy Sommer) #25170 * util: * Inspect ArrayBuffers contents closely (Ruben Bridgewater) #25006 * worker: * Expose workers by default and remove `--experimental-worker` flag (Anna Henningsen) #25361 PR-URL: #25537

nodejs/node#25361

addaleax added the worker Issues and PRs related to Worker support. label Jan 6, 2019

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jan 6, 2019

Trott reviewed Jan 6, 2019

View reviewed changes

benchmark/misc/startup.js Outdated Show resolved Hide resolved

Trott approved these changes Jan 6, 2019

View reviewed changes

hiroppy approved these changes Jan 6, 2019

View reviewed changes

Leko approved these changes Jan 6, 2019

View reviewed changes

jasnell approved these changes Jan 6, 2019

View reviewed changes

cjihrig approved these changes Jan 6, 2019

View reviewed changes

benjamingr approved these changes Jan 6, 2019

View reviewed changes

fixup! worker: remove --experimental-worker flag

e92f055

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 6, 2019

TimothyGu approved these changes Jan 6, 2019

View reviewed changes

tniessen approved these changes Jan 6, 2019

View reviewed changes

shisama approved these changes Jan 7, 2019

View reviewed changes

starkwang approved these changes Jan 7, 2019

View reviewed changes

gireeshpunathil approved these changes Jan 8, 2019

View reviewed changes

mhdawson approved these changes Jan 8, 2019

View reviewed changes

addaleax closed this Jan 8, 2019

addaleax deleted the worker-no-flag branch January 8, 2019 23:44

addaleax added the backport-requested-v11.x label Jan 9, 2019

addaleax added a commit to addaleax/node that referenced this pull request Jan 9, 2019

worker: set --experimental-worker always

18118b2

This is similar to nodejs#25361 in functionality, but allows avoiding some backporting pain for v11.x. Refs: nodejs#25361

addaleax mentioned this pull request Jan 9, 2019

[v11.x] enable workers by default #25404

Closed

4 tasks

BridgeAR added this to Backport requested in v11.x Jan 10, 2019

addaleax removed the backport-requested-v11.x label Jan 15, 2019

BridgeAR mentioned this pull request Jan 16, 2019

v11.7.0 proposal #25537

Merged

addaleax added the notable-change PRs with changes that should be highlighted in changelogs. label Jan 17, 2019

MylesBorins mentioned this pull request Jan 24, 2019

v11.8.0 proposal #25687

Merged

targos removed this from Backport requested in v11.x Jan 30, 2019

sebdeckers added a commit to commonshost/bulldohzer that referenced this pull request Feb 4, 2019

fix: 11.7.0 is where workers were no longer behind experimental flag

b63ef0a

nodejs/node#25361

Suvitruf mentioned this pull request Mar 8, 2019

bcrypt crashes in child process kelektiv/node.bcrypt.js#709

Closed

richardlau mentioned this pull request Jan 30, 2020

src: remove --experimental-worker env option #31577

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

worker: remove `--experimental-worker` flag #25361

worker: remove `--experimental-worker` flag #25361

addaleax commented Jan 6, 2019

nodejs-github-bot commented Jan 6, 2019

addaleax commented Jan 6, 2019

hiroppy left a comment

addaleax commented Jan 6, 2019

gireeshpunathil commented Jan 7, 2019

addaleax commented Jan 8, 2019

gireeshpunathil commented Jan 8, 2019

mhdawson left a comment

addaleax commented Jan 8, 2019

BridgeAR commented Jan 9, 2019

worker: remove --experimental-worker flag #25361

worker: remove --experimental-worker flag #25361

Conversation

addaleax commented Jan 6, 2019

Checklist

nodejs-github-bot commented Jan 6, 2019

addaleax commented Jan 6, 2019

hiroppy left a comment

Choose a reason for hiding this comment

addaleax commented Jan 6, 2019

gireeshpunathil commented Jan 7, 2019

addaleax commented Jan 8, 2019

gireeshpunathil commented Jan 8, 2019

mhdawson left a comment

Choose a reason for hiding this comment

addaleax commented Jan 8, 2019

BridgeAR commented Jan 9, 2019

worker: remove `--experimental-worker` flag #25361

worker: remove `--experimental-worker` flag #25361