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 navigator.hardwareConcurrency #47769

Merged
merged 7 commits into from
Jul 4, 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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ module.exports = {
DecompressionStream: 'readable',
fetch: 'readable',
FormData: 'readable',
navigator: 'readable',
ReadableStream: 'readable',
ReadableStreamDefaultReader: 'readable',
ReadableStreamBYOBReader: 'readable',
Expand Down
37 changes: 37 additions & 0 deletions doc/api/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,41 @@ The `MessagePort` class. See [`MessagePort`][] for more details.

This variable may appear to be global but is not. See [`module`][].

## `Navigator`

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental

An implementation of the [Navigator API][].
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
An implementation of the [Navigator API][].
A patrial implementation of the [Navigator API][].

should we mention this is not a full implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m -0 on this, but since you didn’t block the PR, I recommend merging this PR, if you’re ok with it @MoLow


## `navigator`
anonrig marked this conversation as resolved.
Show resolved Hide resolved

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental

An implementation of [`window.navigator`][].
Copy link
Member

Choose a reason for hiding this comment

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

On a closer look, I think the WorkerNavigator interface might make more sense for us compared to Navigator . Or at least that seems to be a better starting point compared to Navigator. (we already run WPT in workers, too).

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not support both? I suggest adding WorkerNavigator in a different pull request.

Copy link
Member

Choose a reason for hiding this comment

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

I would find it confusing to expose both of them: every Web interface has an Exposed attribute defining the scope where they are available. WorkerNavigator is only available with WorkerGlobalScope and Navigator is only available with WindowGlobalScope. Not both.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. How can I limit window.navigator to window, not worker?

Copy link
Member

@legendecas legendecas Jun 24, 2023

Choose a reason for hiding this comment

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

IIRC, we didn't distinguish WindowGlobalScope and WorkerGlobalScope yet as no WindowGlobalScope specific APIs are exposed.

I'd agree that WorkerNavigator is more limited than Navigator and suits the use case of the PR, since WorkerNavigator doesn't include NavigatorContentUtils, NavigatorCookies, and NavigatorPlugins.

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't really matter now, as we implement the hardwareConcurrency property, which is the same in Navigator and in WorkerNavigator.

But I think we should link to https://developer.mozilla.org/en-US/docs/Web/API/Navigator. The current link points to a useless page IMO, as it only references the userAgent property.


### `navigator.hardwareConcurrency`

<!-- YAML
added: REPLACEME
-->

* {number}

The `navigator.hardwareConcurrency` read-only property returns the number of
logical processors available to the current Node.js instance.

```js
console.log(`This process is running on ${navigator.hardwareConcurrency}`);
```

## `PerformanceEntry`

<!-- YAML
Expand Down Expand Up @@ -998,6 +1033,7 @@ A browser-compatible implementation of [`WritableStreamDefaultWriter`][].

[CommonJS module]: modules.md
[ECMAScript module]: esm.md
[Navigator API]: https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object
[Web Crypto API]: webcrypto.md
[`--no-experimental-fetch`]: cli.md#--no-experimental-fetch
[`--no-experimental-global-customevent`]: cli.md#--no-experimental-global-customevent
Expand Down Expand Up @@ -1057,6 +1093,7 @@ A browser-compatible implementation of [`WritableStreamDefaultWriter`][].
[`setInterval`]: timers.md#setintervalcallback-delay-args
[`setTimeout`]: timers.md#settimeoutcallback-delay-args
[`structuredClone`]: https://developer.mozilla.org/en-US/docs/Web/API/structuredClone
[`window.navigator`]: https://developer.mozilla.org/en-US/docs/Web/API/Window/navigator
[buffer section]: buffer.md
[built-in objects]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects
[module system documentation]: modules.md
Expand Down
4 changes: 4 additions & 0 deletions lib/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ rules:
message: Use `const { MessageEvent } = require('internal/worker/io');` instead of the global.
- name: MessagePort
message: Use `const { MessagePort } = require('internal/worker/io');` instead of the global.
- name: Navigator
message: Use `const { Navigator } = require('internal/navigator');` instead of the global.
- name: navigator
message: Use `const { navigator } = require('internal/navigator');` instead of the global.
- name: PerformanceEntry
message: Use `const { PerformanceEntry } = require('perf_hooks');` instead of the global.
- name: PerformanceMark
Expand Down
6 changes: 5 additions & 1 deletion lib/internal/bootstrap/web/exposed-window-or-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/**
* This file exposes web interfaces that is defined with the WebIDL
* Exposed=(Window,Worker) extended attribute or exposed in
* Exposed=Window + Exposed=(Window,Worker) extended attribute or exposed in
* WindowOrWorkerGlobalScope mixin.
* See more details at https://webidl.spec.whatwg.org/#Exposed and
* https://html.spec.whatwg.org/multipage/webappapis.html#windoworworkerglobalscope.
Expand Down Expand Up @@ -55,6 +55,10 @@ 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']);
anonrig marked this conversation as resolved.
Show resolved Hide resolved
defineReplaceableLazyAttribute(globalThis, 'internal/navigator', ['navigator'], false);

// https://w3c.github.io/FileAPI/#creating-revoking
const { installObjectURLMethods } = require('internal/url');
installObjectURLMethods();
49 changes: 49 additions & 0 deletions lib/internal/navigator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
'use strict';

const {
ObjectDefineProperties,
Symbol,
} = primordials;

const {
ERR_ILLEGAL_CONSTRUCTOR,
} = require('internal/errors').codes;

const {
kEnumerableProperty,
} = require('internal/util');

const {
getAvailableParallelism,
} = internalBinding('os');

const kInitialize = Symbol('kInitialize');

class Navigator {
anonrig marked this conversation as resolved.
Show resolved Hide resolved
// Private properties are used to avoid brand validations.
#availableParallelism;

constructor() {
if (arguments[0] === kInitialize) {
return;
}
throw ERR_ILLEGAL_CONSTRUCTOR();
}

/**
* @return {number}
*/
get hardwareConcurrency() {
anonrig marked this conversation as resolved.
Show resolved Hide resolved
this.#availableParallelism ??= getAvailableParallelism();
return this.#availableParallelism;
}
}

ObjectDefineProperties(Navigator.prototype, {
hardwareConcurrency: kEnumerableProperty,
});

module.exports = {
navigator: new Navigator(kInitialize),
Navigator,
};
8 changes: 8 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,14 @@ if (global.gc) {
knownGlobals.push(global.gc);
}

if (global.navigator) {
knownGlobals.push(global.navigator);
}

if (global.Navigator) {
knownGlobals.push(global.Navigator);
}

if (global.Performance) {
knownGlobals.push(global.Performance);
}
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ builtinModules.forEach((moduleName) => {
'structuredClone',
'fetch',
'crypto',
'navigator',
];
assert.deepStrictEqual(new Set(Object.keys(global)), new Set(expected));
}
Expand Down
15 changes: 15 additions & 0 deletions test/parallel/test-navigator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

require('../common');
const assert = require('assert');

const is = {
number: (value, key) => {
assert(!Number.isNaN(value), `${key} should not be NaN`);
assert.strictEqual(typeof value, 'number');
},
};

is.number(+navigator.hardwareConcurrency, 'hardwareConcurrency');
is.number(navigator.hardwareConcurrency, 'hardwareConcurrency');
assert.ok(navigator.hardwareConcurrency > 0);