lib: add navigator.onLine

[Uzlopak]

Motivated by #50200

I did not add the ability to listen on a status change, as I expect that it is problematical as getting the online status is slow.

Using following benchmark I get 2600 ops/s. I could add to process the events online and offline, if we listen to online or offline. And then i would set a setInterval with lets say 50ms or 250 ms, and check if there was a status change and emit the corresponding event. Maybe if we have listeners, than instead of directly calling getOnlineStatus, we use the last value derived in the setInterval.

But for now, this is an proposal. Maybe you dont like it and we can stomp this PR into the trashbin :P

'use strict';constcommon=require('../common.js');constassert=require('assert');constbench=common.createBenchmark(main,{n: [1e4],});// Warm up.constlength=1024;constarray=[];for(leti=0;i<length;++i){array.push(global.navigator.onLine);}functionmain({ n }){bench.start();for(leti=0;i<n;++i){constindex=i%length;array[index]=global.navigator.onLine;}bench.end(n);// Verify the entries to prevent dead code elimination from making// the benchmark invalid.for(leti=0;i<length;++i){assert.strictEqual(typeofarray[i],'boolean');}}

[Uzlopak]

I investigated getifaddrs used by libuv is just horribly slow. I dont think that fast path makes a difference, when the underlying function call is simply too slow.

[tniessen]

As explained in #39540, I doubt that this is helpful. The existence of non-loopback interfaces does not imply that the device is online.

I understand that for some web applications, being able to detect when the user's (mobile) device has lost connectivity might be important. Also, some browsers allow users to explicitly enable an "offline mode," in which case navigator.onLine also returns false.

However, even in browsers, this is widely unreliable, which is why it is often coupled with actual connectivity checks. The only benefit of navigator.onLine is avoiding additional checks if the property is false. However, if the property is false, then any additional attempt at connecting to a remote resource (e.g., through fetch()) should fail immediately. In other words, even in the unlikely scenario that navigator.onLine is false, the benefit is marginal.

Further, only a fraction of Node.js deployments are on mobile devices that might benefit from this. Enterprise deployments on servers as well as within containers will likely have network setups that will always cause this property to be true.

In command-line tools and such, checking navigator.onLine before fetching some remote resource is not good practice either, similar to how calling fs.exists before reading a file is bad practice.

This implementation is also not spec-compliant unless the relevant events are fired as described in the HTML standard. (We don't have to implement any properties from the HTML standard. The whole navigator object was designed for browsers, not for server-side runtimes. But if we do implement some of those properties, they should at least be spec-compliant.)

On the other hand, always returning true would be spec-compliant and not much less useful than this implementation. It eliminates the need for firing events because the value never changes. (Of course, I'm not actually in favor of adding such a property.)

[Uzlopak]

I am not pushing this. I am just proposing. Maybe it helps to make a final decision regarding navigator being fully implemented or being ripped out.

[GeoffreyBooth]

Let’s please ping @nodejs/web-standards for all issues and PRs related to navigator.

[mcollina]

lgtm

[jasnell]

I have to agree with @tniessen's view on this one not being entirely useful and inherently unreliable for most Node.js use cases.

[GeoffreyBooth]

I am not pushing this. I am just proposing. Maybe it helps to make a final decision regarding navigator being fully implemented or being ripped out.

I don’t think “fully implemented” is something that’s being considered. Bun and Deno each only implement a handful of navigator properties, and among browsers there are certain properties that only some browsers support. I think Node should implement as many as possible for compatibility reasons, just to help allow as much browser code as possible run server-side successfully; but we shouldn’t implement properties that we just can’t implement successfully. As in, we shouldn’t add a property with a dummy value just so that some browser code runs without throwing a ReferenceError; if the browser code is actually doing something with the value, that part should work too.

[legendecas]

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

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

[jasnell]

I'm not at all convinced this should be added. While the conversation is pending, I'm going to add an explicit -1 to ensure this doesn't get landed in the meantime.