lib: add navigator legacy attributes

[Uzlopak]

This PR adds the "dummy" attributes to the navigator instance, like navigator.product or navigator.appName.

I know this PR will get alot of heat and I am just providing this just for the sake of completeness and for discussion.

[nodejs-github-bot]

Review requested:

• @nodejs/web-standards

[GeoffreyBooth]

I think “legacy” is a better name for these than “dummy.” They’re basically harmless, kept around to avoid breaking very old scripts; though I wonder how many such scripts anyone would actually likely try to run in Node.

navigator.appVersion is actually legitimately useful, as it’s not a static fixed value. In Chrome it isn’t identical to navigator.userAgent:

navigator.userAgent
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118.0.0.0 Safari/537.36'
navigator.appVersion
'5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118.0.0.0 Safari/537.36'

[Uzlopak]

To be honest the appVersion was the most dubios attribute of them all. It is not clear what the "version" is actually. What does the version 5.0 refer to? You use Google Chrome v118.0.0 but it says version 5.0?

In the spec it says use version "4.0" or the correct version. But it seems to me that version 5.0 refers to the supported html version, which is html5.

But we dont have html support anyway. So I just added the trail, which is basically the same as userAgent.

wdyt

[GeoffreyBooth]

If you’re following the spec for appVersion then I think that’s all we need to do.

Sure let’s land these, but maybe open an issue to discuss what remaining navigator properties/APIs you want to add? After #50303 we will have added everything that Deno and Bun have. Arguably the only property we really needed was userAgent, so we could stop at any time, but there’s so much on https://developer.mozilla.org/en-US/docs/Web/API/Navigator that we should consider which (if any) others we want.

[tniessen]

I wonder if it is a good idea to bloat the documentation with all these properties that are standardized and that nobody should use.

This property is kept only for compatibility purposes.

Do we have any empirical data about this? It looks like none of the other server runtimes implemented these properties.

[tniessen]

Suggested change

	> Stability: 3 - Legacy.

[tniessen]

Suggested change

	> Stability: 3 - Legacy.

[tniessen]

Suggested change

	> Stability: 3 - Legacy.

[tniessen]

Suggested change

	> Stability: 3 - Legacy.

[tniessen]

Suggested change

	> Stability: 3 - Legacy.

[tniessen]

Suggested change

	> Stability: 3 - Legacy.

[tniessen]

Suggested change

	> Stability: 3 - Legacy.

[legendecas]

Filed wintercg/proposal-common-minimum-api#61 to discuss the inclusion of legacy attributes.

(Note: Node.js didn't declare itself to be a WinterCG runtime at the moment so this is not a blocker)

[GeoffreyBooth]

@tniessen The overall API is defined as experimental so we I think we shouldn’t declare individual properties as legacy until at least the overall API is stable. We could mention that their use is discouraged in general, but generally try to avoid repeating what MDN covers.

[KhafraDev]

These properties have no purpose being in node, even for compatibility reasons. Plus, having them as part of navigator -- a global -- unflagged & not marked as legacy encourages their use.

[Uzlopak]

To be honest, i am not like this PR would be the shining part of my CV :D. I just provided this PR for the sake of completeness. And if we decide that it is a bad idea, then we can just close it.

Regarding the documentation i agree. I just followed the usual pattern. Because of the remarks i think we can simplify it and put it into a paragraph and a list the attributes in a two column table, with key and value.

[GeoffreyBooth]

I just provided this PR for the sake of completeness.

We’re unlikely to ever implement all of the navigator APIs that browsers do, so I think we can ignore these until that potentially changes someday and these are the last ones remaining unimplemented. I think the only thing that would tip the scale earlier would be some popular library that relied on these: then there would be a concrete benefit to adding them.

Why don’t you open an issue where we can discuss what, if any, other navigator APIs we should add? Then you’ll get some guidance before putting in the effort of making a PR.

[Uzlopak]

@GeoffreyBooth

I get what you say. But it helps to see the actual code and what the shape of it will be. If we discuss it abstract we see maybe the issues in the final PR. So it is better to see, what we actually talking about. Also tbh. I dont see it as wasted effort. It helps me to orientate better in the nodejs codebase and improving my skills ;).

[GeoffreyBooth]

it helps to see the actual code and what the shape of it will be.

Sure, whatever you want. I know for myself I find it frustrating to invest time into a PR that never lands.

Looking at https://developer.mozilla.org/en-US/docs/Web/API/Navigator, the APIs that I would be most interested in are:

• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/clipboard - could be useful for a CLI app
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/cookieEnabled - I guess would always be false until we add general support for cookies
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/locks and the Web Locks API in general could be very useful for workers
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/maxTouchPoints - should just return 0, would be good to have for compatibility for server-side rendering
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/onLine - should just return true, I’d guess, for compatibility for server-side rendering
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon - this is a popular API used by many analytics libraries, so it would be good to implement this to improve compatibility
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/serviceWorker - more useful stuff for workers
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/share / https://developer.mozilla.org/en-US/docs/Web/API/Navigator/canShare - could pop up the OS Share dialog (which is supported by Windows and macOS) which could be useful for a CLI app
• https://developer.mozilla.org/en-US/docs/Web/API/Navigator/storage - part of deps,doc,lib,src,test: add experimental web storage #50169

[jasnell]

I really have to question adding these. They are not implemented by other non-browser runtimes so will not benefit interop and they really serve no purpose otherwise. Just because we added navigator it doesn't mean we have to support everything that exists in the browser navigator object.

[Uzlopak]

@GeoffreyBooth
@legendecas
@jasnell
I am totally fine with closing this PR. Or should we wait for feedback from wintercg?

[GeoffreyBooth]

I am totally fine with closing this PR. Or should we wait for feedback from wintercg?

It doesn’t really matter either way. We can always reopen.

I think what would tip the scale toward wanting to land this is if any of these APIs were actually used by some library for some purpose, where it helps compatibility for Node to support them. Like if there’s some realistic client-side library that can’t run in Node without these but could run with these, then there’s a clear reason why adding them is beneficial. But absent at least one such example, it seems like we’re adding maintenance burden for no benefit.

[Uzlopak]

I agree ;)