Using package in Bun or in Deno with `npm:` specifier reports `isNode` as `true`

Environment

Package version:

Deno:

version: 1.38.2
OS: Windows

Reproduction

In a Deno REPL, run the following:

import {isNode, isDeno} from 'https://esm.sh/std-env';
console.log(isNode, isDeno); // false true

import {isNode, isDeno} from 'npm:std-env';
console.log(isNode, isDeno); // true true

Describe the bug

Under Deno, when using the package in a "regular" way, the platform detection works as expected.

But when using the npm: specifier, isNode is set to true.

If you log the process object exported by the package, you can see the difference: package loaded using URL import has an empty one, while the one loaded with npm has a "regular Node.js" process object, leading to the false positive.

Additional context

It would be feasible to correct that false positive by checking if isDeno is true, then forcing isNode to false. I guess it would be required under Bun as well since it provides Node.js compatibility too.

However, that leads to a question of semantics: do you want the library to report an identity, or a compatibility? Flags starting with is suggest identity, hence my opinion of thinking of this as a bug. However, you could extend the features of your package by also adding flags for compatibility, e.g. isLikeNode, etc.

However the latter would be very tricky: in my tests, globalThis.process, which you use for detection, is undefined except inside the context of evaluation of the package loaded with the npm: specifier. So the package could report a positive Node.js compatibility, when it's not 100% the case: the user has to import the process object here with import process from 'node:process', which is still making a difference. Besides that, Deno embeds Node.js compatibility but is more strict, for instance the node: specifier in imports is required.

I believe it should be double checked for Bun too. EDIT: I checked Bun (on WSL) and it really pushed Node.js' compatibility further: not only process is always global (and process.release.name does equal "node"), but it also doesn't force using node: specifier when importing builtin Node.js modules. And I confirm it also reports isNode as true.

Hi, and thanks for writing a detailed issue.

In Node.js compatibility mode, I guess Deno tries to mimic Node.js env and set process.release.name to node.

I understand that you want to strictly be sure about current runtime and this is a really ecosystem wide confusing issue.

It will be tricky to decide for Bun and Deno what expect in compatibility mode to be detected-as, they like libraries to magically assume them Node or not. We probably need to reach out.

It is also a breaking change for std-env in regards of detection.

What i suggest to do, is to introduce some new strict exports like isDenoRuntime, isBunRuntime + realRuntime export to prefers them (strange, i know!).

In Node.js compatibility mode, I guess Deno tries to mimic Node.js env and set process.release.name to node.

When entering that mode, which is the case when loading packages with the npm: specifier, it indeed sets the release name to node, besides also making the process object available on the global scope.

I understand that you want to strictly be sure about current runtime and this is a really ecosystem wide confusing issue.

It's more about how this projects decides to define the semantics of the flags it computes. Then consumers of the API will simply follow what's documented.

It will be tricky to decide for Bun and Deno what expect in compatibility mode to be detected-as, they like libraries to magically assume them Node or not. We probably need to reach out.

Right now I see this library as doing identity checking rather than duck testing, so that's why I was personally expecting the flags to tell what the platform truly is (also because of the naming scheme of the flags). Duck testing would be more complex and demanding, but if done correctly would manage to identify that Deno is not Node.js compatible out of the box (AFAIU) while Bun is.

It is also a breaking change for std-env in regards of detection.

That, I think, is an open debate.

"breaking change" refers to semantic versioning, and here we are potentially talking about a bug fix, which is not considered as a breaking change in this context. But when thinking more concretely, yes it is a breaking change. However we can always find a way to consider "any" change a breaking change since it modifies the context of a program. Changing the content of the code you distribute is a breaking change if a consumer, for any reason at all, decides to read the raw code and do some things about it. That sounds silly but why not. However, the precise layout of the distributed code is not considered as part of the public contract for this project, hence why no one considers that as a breaking change.

"breaking change" or not should always be considered with regards to the explicit contract, or in other words the public documentation, and therefore the semantics of the API. In that case, if you always defined the platform detection as being strict (i.e. "it is" vs "it is like/compatible with"), then we're talking about a bug fix. A fix that would indeed break the programs of the consumers if they were relying on that bug. But I personally really doubt projects should start considering cases like that otherwise it becomes a nightmare to handle and all releases would be a major version bump.

What i suggest to do, is to introduce some new strict exports like isDenoRuntime, isBunRuntime + realRuntime export to prefers them (strange, i know!).

Not sure I understand the suggestion here. So isNodeRuntime would be true only if it's really Node.js, while isNode would be true when other runtimes like Bun or Deno run with Node.js compatibility active? I'm not against two sets of flags with different semantics, but I think that this precise naming here will start getting really confusing.

Hi dear @ymeine thanks again for bringing this up. Thinking more and considering the situation that more runtimes are deciding to have a Node.js compatibility mode, I think it is finally up to them to make sure their runtimes are Node.js compatible when they explicitly say release.name is node is is far beyond std-env's scope.

We have a new fix for Bun runtime check (#107) and I have also clarified docs/jsdocs about isNode flag behavior which indicates a Node.js compatible runtime. (#108)

For strict Node.js checking runtime === "node" can be used in place and we shall guarantee it will work as is.

Hi, thanks for the update and the fixes!

I think the documentation clarification is the essential part so it's good that it's here now. If ever the behavior would change again (due to updates external to this project), people should now be more alert about what may have cause it.

I personally still think the name isNode is confusing and wrong now that we know some platforms act like Node.js (but aren't Node.js, and often aren't 100% truly compatible), but on the other hand I understand the intention not to disrupt too much the API which has been designed, documented and adopted by users of this library. And anyways, at some point if people need to know for sure if a platform IS** Node.js, you documented how to do it. No need indeed to break everything if people are eventually adhering to the behavior, we'll see with time.

unjs / std-env