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.

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

lib: add --no-experimental-global-navigator CLI flag #50562

Merged
merged 2 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,16 @@ added: v19.0.0

Disable exposition of [CustomEvent Web API][] on the global scope.

### `--no-experimental-global-navigator`

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental
aduh95 marked this conversation as resolved.
Show resolved Hide resolved

Disable exposition of [Navigator API][] on the global scope.

### `--no-experimental-global-webcrypto`

<!-- YAML
Expand Down Expand Up @@ -2350,6 +2360,7 @@ Node.js options that are allowed are:
* `--no-deprecation`
* `--no-experimental-fetch`
* `--no-experimental-global-customevent`
* `--no-experimental-global-navigator`
* `--no-experimental-global-webcrypto`
* `--no-experimental-repl-await`
* `--no-extra-info-on-fatal-exception`
Expand Down Expand Up @@ -2765,6 +2776,7 @@ done
[Module customization hooks]: module.md#customization-hooks
[Module customization hooks: enabling]: module.md#enabling
[Modules loaders]: packages.md#modules-loaders
[Navigator API]: globals.md#navigator
[Node.js issue tracker]: https://github.com/nodejs/node/issues
[OSSL_PROVIDER-legacy]: https://www.openssl.org/docs/man3.0/man7/OSSL_PROVIDER-legacy.html
[Permission Model]: permissions.md#permission-model
Expand Down
15 changes: 5 additions & 10 deletions doc/api/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,8 @@ This variable may appear to be global but is not. See [`module`][].
added: v21.0.0
-->

> Stability: 1.1 - Active development
> Stability: 1.1 - Active development. Disable this API with the
> [`--no-experimental-global-navigator`][] CLI flag.

A partial implementation of the [Navigator API][].

Expand All @@ -610,18 +611,11 @@ A partial implementation of the [Navigator API][].
added: v21.0.0
-->

> Stability: 1.1 - Active development
> Stability: 1.1 - Active development. Disable this API with the
> [`--no-experimental-global-navigator`][] CLI flag.

A partial implementation of [`window.navigator`][].

If your app or a dependency uses a check for `navigator` to determine whether it
is running in a browser, the following can be used to delete the `navigator`
global before app code runs:

```bash
node --import 'data:text/javascript,delete globalThis.navigator' app.js
```

### `navigator.hardwareConcurrency`

<!-- YAML
Expand Down Expand Up @@ -1145,6 +1139,7 @@ A browser-compatible implementation of [`WritableStreamDefaultWriter`][].
[Web Crypto API]: webcrypto.md
[`--experimental-websocket`]: cli.md#--experimental-websocket
[`--no-experimental-global-customevent`]: cli.md#--no-experimental-global-customevent
[`--no-experimental-global-navigator`]: cli.md#--no-experimental-global-navigator
[`--no-experimental-global-webcrypto`]: cli.md#--no-experimental-global-webcrypto
[`AbortController`]: https://developer.mozilla.org/en-US/docs/Web/API/AbortController
[`ByteLengthQueuingStrategy`]: webstreams.md#class-bytelengthqueuingstrategy
Expand Down
4 changes: 0 additions & 4 deletions lib/internal/bootstrap/web/exposed-window-or-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ exposeLazyInterfaces(globalThis, 'perf_hooks', [

defineReplaceableLazyAttribute(globalThis, 'perf_hooks', ['performance']);

// https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object
exposeLazyInterfaces(globalThis, 'internal/navigator', ['Navigator']);
defineReplaceableLazyAttribute(globalThis, 'internal/navigator', ['navigator'], false);

// https://w3c.github.io/FileAPI/#creating-revoking
const { installObjectURLMethods } = require('internal/url');
installObjectURLMethods();
14 changes: 14 additions & 0 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ function prepareExecution(options) {
const mainEntry = patchProcessObject(expandArgv1);
setupTraceCategoryState();
setupInspectorHooks();
setupNavigator();
setupWarningHandler();
setupUndici();
setupWebCrypto();
Expand Down Expand Up @@ -336,6 +337,19 @@ function setupUndici() {
}
}

// TODO(aduh95): move this to internal/bootstrap/web/* when the CLI flag is
// removed.
function setupNavigator() {
if (getEmbedderOptions().noBrowserGlobals ||
getOptionValue('--no-experimental-global-navigator')) {
return;
}

// https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object
exposeLazyInterfaces(globalThis, 'internal/navigator', ['Navigator']);
defineReplaceableLazyAttribute(globalThis, 'internal/navigator', ['navigator'], false);
}

// TODO(aduh95): move this to internal/bootstrap/web/* when the CLI flag is
// removed.
function setupWebCrypto() {
Expand Down
5 changes: 5 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
&EnvironmentOptions::experimental_global_customevent,
kAllowedInEnvvar,
true);
AddOption("--experimental-global-navigator",
"expose experimental Navigator API on the global scope",
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for adding this noop? It doesn't seem to be used, or mentioned in anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a no-op, it's the flag being added; see the above one for an another example of such flag:

node/src/node_options.cc

Lines 389 to 393 in 3860002

AddOption("--experimental-global-customevent",
"expose experimental CustomEvent on the global scope",
&EnvironmentOptions::experimental_global_customevent,
kAllowedInEnvvar,
true);

That being said, it looks like I forgot to switch the default value to true

&EnvironmentOptions::experimental_global_navigator,
kAllowedInEnvvar,
true);
AddOption("--experimental-global-webcrypto",
"expose experimental Web Crypto API on the global scope",
&EnvironmentOptions::experimental_global_web_crypto,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ class EnvironmentOptions : public Options {
bool experimental_fetch = true;
bool experimental_websocket = false;
bool experimental_global_customevent = true;
bool experimental_global_navigator = true;
bool experimental_global_web_crypto = true;
bool experimental_https_modules = false;
bool experimental_wasm_modules = false;
Expand Down