lib: add the Navigator object #39581

Mesteery · 2021-07-29T21:59:57Z

Add the Navigator object with the Device Memory API.

Fixes: #39540
Refs: https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object
Refs: https://w3c.github.io/device-memory

I couldn't add the WPT for Navigator, because they all failed (mainly because they depend a lot on the browser environment and the implementation is not complete).
By the way, I would like to know if the tests are sufficient.

doc/api/globals.md

test/common/index.js

Add the Navigator object with the Device Memory API. Refs: nodejs#39540 Refs: https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object Refs: https://w3c.github.io/device-memory Co-authored-by: Voltrex <mohammadkeyvanzade94@gmail.com>

aduh95 · 2021-07-29T22:35:27Z

Can you add an entry in the eslint list of restricted globals please:

    - name: navigator
      message: "Use `const navigator = require('internal/navigator');` instead of the global."

node/lib/.eslintrc.yaml

Lines 78 to 79 in 7919ced

    
           - name: performance 
        
             message: "Use `const { performance } = require('perf_hooks');` instead of the global."

jasnell · 2021-07-29T23:17:06Z

I'm not convinced we should add this. Yes, there are a lot of web platform APIs that do make sense but I'm not convinced yet that this is one of them. Can you go into more detail on the expected use case and justification?

Mesteery · 2021-07-30T11:41:53Z

The Navigator object is interesting because it brings together many APIs that can be useful, even in the context of Node.js, such as the Network Information API, the WebUSB API, and the Geolocation API, for example.
However, I agree that at the moment (with this PR) it is less interesting.
When these APIs are implemented over time, Navigator will become, in my opinion, very interesting and useful.

tniessen · 2021-07-30T16:16:03Z

The Navigator object is interesting because it brings together many APIs that can be useful, even in the context of Node.js

Navigator is defined by the HTML standard, not by the ECMAScript standard, and I am afraid most of its features are not necessarily meaningful in Node.js.

Navigator fields according to the current HTML standard:

NavigatorID: these values must be provided by each Node.js application and cannot be provided by Node.js itself. Some properties (e.g., userAgent) do not really make sense outside of web browsers as far as I can tell.
NavigatorLanguage: we could maybe use the system language here, but that is not necessarily the "user's preferred language." The user accessing the application might not be the same user that is running the Node.js process.
NavigatorOnLine: this does not seem to make sense for Node.js applications that might be connected to multiple networks through multiple network interfaces.
NavigatorContentUtils: this does not seem useful in Node.js.
NavigatorCookies: this does not seem to make sense in Node.js (neither true nor false is a meaningful value).
NavigatorPlugins: ouch
NavigatorConcurrentHardware: this is an indication at best and less useful than os.cpus().

Suggested extensions:

NavigatorDeviceMemory: this does not seem helpful, and the use cases covered by the "Introduction" section in the draft do not apply to Node.js. More precise APIs exist in Node.js.
NavigatorNetworkInformation: this seems to be pretty much useless for any server-side application because it is impossible for Node.js to tell what network interface the information should be referring to.
USB: this might actually maybe be a useful feature.
Geolocation: not sure if useful or how to implement this on a server.

Mesteery · 2021-07-30T17:20:18Z

Navigator is defined by the HTML standard, not by the ECMAScript standard

I know.

NavigatorContentUtils: this does not seem useful in Node.js.
NavigatorCookies: this does not seem to make sense in Node.js (neither true nor false is a meaningful value).
NavigatorPlugins: ouch
NavigatorID: these values must be provided by each Node.js application and cannot be provided by Node.js itself. Some properties (e.g., userAgent) do not really make sense outside of web browsers as far as I can tell.
NavigatorLanguage: we could maybe use the system language here, but that is not necessarily the "user's preferred language." The user accessing the application might not be the same user that is running the Node.js process.

Yes, I agree, and they are the ones I mean when I say they should not be implemented.

NavigatorDeviceMemory: this does not seem helpful, and the use cases covered by the "Introduction" section in the draft do not apply to Node.js. More precise APIs exist in Node.js.

Personally I find it interesting to have the RAM rounded to the nearest power of 2. But not having a lower bound of 0.25 and upper bound of 8 GiB as proposed in the specification.
And I don't think this information is imprecise. In my opinion it is not uncommon to need this rounded device memory.

NavigatorOnLine: this does not seem to make sense for Node.js applications that might be connected to multiple networks through multiple network interfaces.

Even though a Node.js application can be connected to multiple networks through multiple network interfaces, false can be returned if the application is completely disconnected.

Geolocation: not sure if useful or how to implement this on a server.

If the machine does not have GPS, the errorCallback can be called with the error https://www.w3.org/TR/geolocation/#dom-geolocationpositionerror-position_unavailable. It's true that it's hard to find a usecase for this one, but I think there's probably a useful purpose for this, for example a program that returns the current GPS location.

We can, not implement APIs that are not useful in Node.js, while having Navigator and some useful APIs.

I also think we should discuss this in the issue, and possibly get the author's opinion, right?

tniessen · 2021-07-31T13:46:38Z

I also think we should discuss this in the issue, and possibly get the author's opinion, right?

Good idea, see #39540 (comment).

devsnek · 2021-07-31T13:59:05Z

i'm not wholly opposed to this but i'd like to see what implementation of one of the more novel apis looks like. cpu count and platform alone don't carry this imo. we also need to check there isn't code that uses navigator to assume its in a browser (i would not expect that there is, but we should be careful).

Mesteery · 2021-08-01T18:29:43Z

doc/api/globals.md

+console.log(`This device has ${navigator.hardwareConcurrency} logical CPUs`);
+```
+
+### `navigator.platform`


The specification does not indicate any deprecation.

Hmm 😕
Confused... Seems like MDN and TypeScript have incorrectly marked it as deprecated?

https://stackoverflow.com/questions/38506517/deprecated-javascript-os-detection-techniques

@jimmywarting MDN takes the term "deprecated" to mean "use of this API is discouraged" - not "this API is on the way to removal from the spec". Under MDN's meaning of the term this marking as deprecated is correct because most of the properties/methods return incorrect and/or inconsistent information. There are more reliable ways to get the information.

Of course it would be good if the reasoning was provided along with the tagging.

Note that around https://html.spec.whatwg.org/multipage/system-state.html#dom-navigator-oscpu, the spec adds this big red warning about the entire set of NavigatorID members (including platform):

Any information in this API that varies from user to user can be used to profile the user. In fact, if enough such information is available, a user can actually be uniquely identified. For this reason, user agent implementers are strongly urged to include as little information in this API as possible.

That — along with the fact that most of the members are useless because they’re required by the spec to either not ever return anything useful or to not reliably/deterministically return anything useful — is why we have it flagged as deprecated in MDN: As a “don’t use this” warning to developers, so we don’t end up causing developers to mistakenly try to use any of it.

For example, in the case of platform, the spec requirements are:

Must return either the empty string or a string representing the platform on which the browser is executing, e.g. "MacIntel", "Win32", "FreeBSD i386", "WebTV OS".

So, given that browsers can choose to always return the empty string for it, that means developers cannot and should not write code which assumes it returns "MacIntel", "Win32", "FreeBSD i386", "WebTV OS" or anything else actually useful. So it’s flagged as deprecated in MDN to help developers avoid making that mistake.

Unless the spec have marked it as deprecated or removed it then i think you should use the term "Legacy" instead of "Deprecated" like NodeJS docs dose or something like that.
Think "Deprecated" is a too strong word for what it actually means.

Say something in terms of: "Legacy discouraged fingerprinting, use feature detection instead"

anonrig · 2023-04-28T16:17:12Z

How does @nodejs/tsc stand in regards of this pull request? I see that it was abandoned, and if there are no objections/blockers I'd like to take over and add a simple implementation starting with navigator.platform.

bnoordhuis · 2023-06-02T14:10:50Z

I'm going to close this in favor of #47769. This PR looks abandoned and has many merge conflicts.

nodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jul 29, 2021

VoltrexKeyva reviewed Jul 29, 2021

View reviewed changes

doc/api/globals.md Outdated Show resolved Hide resolved

VoltrexKeyva reviewed Jul 29, 2021

View reviewed changes

test/common/index.js Outdated Show resolved Hide resolved

lib: add the Navigator object

4b158f1

Add the Navigator object with the Device Memory API. Refs: nodejs#39540 Refs: https://html.spec.whatwg.org/multipage/system-state.html#the-navigator-object Refs: https://w3c.github.io/device-memory Co-authored-by: Voltrex <mohammadkeyvanzade94@gmail.com>

Mesteery force-pushed the introduce-navigator branch from e21f84c to 4b158f1 Compare July 29, 2021 22:09

tniessen mentioned this pull request Jul 31, 2021

Add a global Navigator object #39540

Closed

targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Aug 1, 2021

jimmywarting reviewed Aug 1, 2021

View reviewed changes

jimmywarting mentioned this pull request Aug 1, 2021

More updates to mark pages as deprecated mdn/content#4226

Merged

Trott force-pushed the main branch from 2d76238 to ca3ed36 Compare November 12, 2022 01:49

anonrig mentioned this pull request Apr 28, 2023

lib: add navigator.hardwareConcurrency #47769

Merged

bnoordhuis closed this Jun 2, 2023

Mesteery deleted the introduce-navigator branch June 2, 2023 16:04

targos mentioned this pull request Oct 19, 2023

navigator object breaks projects in v21.0.0 #50269

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: add the Navigator object #39581

lib: add the Navigator object #39581

Mesteery commented Jul 29, 2021 •

edited by aduh95

Loading

aduh95 commented Jul 29, 2021

jasnell commented Jul 29, 2021

Mesteery commented Jul 30, 2021 •

edited

Loading

tniessen commented Jul 30, 2021

Mesteery commented Jul 30, 2021 •

edited

Loading

tniessen commented Jul 31, 2021

devsnek commented Jul 31, 2021

This comment was marked as resolved.

Mesteery Aug 1, 2021

jimmywarting Aug 1, 2021

hamishwillee Aug 1, 2021

sideshowbarker Aug 1, 2021 •

edited

Loading

jimmywarting Aug 2, 2021

anonrig commented Apr 28, 2023

bnoordhuis commented Jun 2, 2023

lib: add the Navigator object #39581

lib: add the Navigator object #39581

Conversation

Mesteery commented Jul 29, 2021 • edited by aduh95 Loading

aduh95 commented Jul 29, 2021

jasnell commented Jul 29, 2021

Mesteery commented Jul 30, 2021 • edited Loading

tniessen commented Jul 30, 2021

Mesteery commented Jul 30, 2021 • edited Loading

tniessen commented Jul 31, 2021

devsnek commented Jul 31, 2021

This comment was marked as resolved.

Mesteery Aug 1, 2021

Choose a reason for hiding this comment

jimmywarting Aug 1, 2021

Choose a reason for hiding this comment

hamishwillee Aug 1, 2021

Choose a reason for hiding this comment

sideshowbarker Aug 1, 2021 • edited Loading

Choose a reason for hiding this comment

jimmywarting Aug 2, 2021

Choose a reason for hiding this comment

anonrig commented Apr 28, 2023

bnoordhuis commented Jun 2, 2023

Mesteery commented Jul 29, 2021 •

edited by aduh95

Loading

Mesteery commented Jul 30, 2021 •

edited

Loading

Mesteery commented Jul 30, 2021 •

edited

Loading

sideshowbarker Aug 1, 2021 •

edited

Loading