Native class of global changed in Node v7

[kgryte]

• Version: 7.0
• Platform: Darwin local 14.5.0 Darwin Kernel Version 14.5.0: Thu Apr 21 20:40:54 PDT 2016; root:xnu-2782.50.3~1/RELEASE_X86_64 x86_64

In Node.js version <= 6.x.x,

var nativeClass = Object.prototype.toString.call( global );
// returns '[object global]'

In Node.js version 7.0,

var nativeClass = Object.prototype.toString.call( global );
// returns '[object Object]'

Based on the changelog, this is not listed as a notable and/or breaking change. Why did the internal class change and is this a permanent/intentional change?

Background: one reason why this is a notable change is that environment sniffers (i.e., packages which try and detect if the runtime is Node versus, say, a browser) commonly use the internal class of global as means to identify a Node environment.

[MylesBorins]

Digging in

/cc @nodejs/ctc

[jasnell]

I'm not sure if this is a change that we made or if it came from the v8 update.

[addaleax]

I can start bisecting this

[bnoordhuis]

It's a change that comes from V8. We don't muck around with the global object's prototype. Example with v6:

> vm.runInNewContext('Object.prototype.toString.call(this)')
'[object global]'

With v7:

> vm.runInNewContext('Object.prototype.toString.call(this)')
'[object Object]'

[ofrobots]

/cc @nodejs/v8

[MylesBorins]

I can confirm that this change came in somewhere between 8a24728...96933df

which leads me to believe this came with the V8 upgrade

[addaleax]

Btw, this can be fixed with global[Symbol.toStringTag] = 'global'. I’m not sure whether that should be applied to any of the objects generated by the vm module, though.

[bnoordhuis]

Can, but I don't know if we should. If the use case is feature detection, then sniffing for process seems like a better choice in the first place.

[kgryte]

@bnoordhuis A use case is feature detection. Another time when the internal class matters is when type checking or handling special cases; e.g.,

var toString = Object.prototype.toString;

function foo( bar ) {
   var nativeClass = toString.call( bar );
   if ( nativeClass === '[object global]' ) {
     // ...do something...
   } else if ( nativeClass === '[object process]' ) {
     // ...do something...
   } else {
     // ...do something else
   }
}

And specifically re: environment detection: usually checking the internal class is combined with other checks including process in order to more confidently claim an environment is Node.

Regardless, the change introduces an (unintended) inconsistency between Node versions.

[addaleax]

I think I agree; if something like this is changed, it should be conscious decision we make.

(I also agree that sniffing for process seems like the best way to detect Node.)

[mscdex]

I would go a step further and say that checking that process.versions.node exists (checking each property in the path exists of course) and is a non-empty string is all that is really needed.

[bnoordhuis]

@kgryte My point is more that checking for this particular behavior is not a very good indicator of a node.js runtime because you'd see the same behavior in, say, plv8, or any V8-backed runtime that doesn't override the default.

[kgryte]

Just to clarify and keep things focused: this issue is not about the right way to detect a Node environment, but about the fact that an unintended breaking change was introduced in Node v7 due to how V8 reports the internal class of global.

[ljharb]

This change should be kept. Object#toString is simply not a reliable brand check anymore (since ES6 and Symbol.toStringTag) and any code relying on its output is automatically broken.

[ljharb]

@kgryte I did extensive research for the global proposal and found exactly zero instances of using the output of Object#toString to identify the global object - could you perhaps link to any?

[kgryte]

@ljharb I am not aware of public npm packages. Here is a Stack Overflow post in which the '[object global]' check is proposed (among others). I have never seen a foolproof approach to detecting a Node.js runtime (even the process check proposed above is not foolproof). Nevertheless, I have seen code at two separate companies which use the '[object global]' check, along with other checks such as '[object process]' and process.versions.node and other properties of the process object.

But once again, whether or not Object#toString is a reliable brand check is secondary to the fact that a change in Node behavior did occur which was not intentional/planned. I filed the issue because I was affected by this change (builds broke when running on Node.js v7).

Simply because Symbol.toStringTag allows people to affect the return value of Object#toString does not mean that the global variable in Node should return inconsistent values across different Node versions. The introduction of this change means that people who do have an '[object global]' check need to now split based on Node version (as the check is still valid in environments lacking Symbol.toStringTag support, which include Node.js v4 without the --harmony flag).

[dnalborczyk]

The introduction of this change means that people who do have an '[object global]' check need to now split based on Node version (as the check is still valid in environments lacking Symbol.toStringTag support, which include Node.js v4 without the --harmony flag).**

Yes, only if they really want to keep that particular code in place, which would be unwise. Instead, affected code could just be corrected and "rely" on other means of 'environment' checks as mentioned above, 'process' etc. Though it's still not reliable as anyone can create, overwrite or delete those objects - if fooling someone is intended.

Though I wonder why anyone would need to know in what environment their code runs - even 'process' in node.js doesn't guarantee the existence or behavior of anything - as node.js is thankfully evolving, and so is v8, as well as browsers.

[kgryte]

Though I wonder why anyone would need to know in what environment their code runs

@dnalborczyk As someone who writes packages to work in Node, the browser, and electron, I regularly attempt to detect the runtime environment. For example, when running tests during CI, I often want to run a particular subset of tests only in a Node environment. Other times, I check because I want to expose a main export which is environment dependent. And yet other times I internally want to know the environment so I can branch accordingly; e.g., cluster-based numeric computing where I either want to use browser web workers or the cluster package.

But once again, an '[object global]' check is often not used in isolation. I have encountered it being used in conjunction with other checks to more reliably detect an environment. For example,

function isNode() {
    return (
        typeof global === 'object' &&
        global === global.global &&
        Object.prototype.toString.call( global ) === '[object global]' &&
        typeof require === 'function' &&
        typeof process === 'object' &&
        Object.prototype.toString.call( process ) === '[object process]' &&
        typeof process.versions === 'object' &&
        typeof process.versions.node === 'string' &&
        (
            typeof process.release === 'undefined' ||
            (
                typeof process.release === 'object' &&
                typeof process.release.name === 'string' &&
                /node|io\.js/.test( process.release.name )
            )
        )
    );
}

So, yes, even the above is not foolproof, as someone could "fake" any of the above globals, pseudo-globals, properties, and values. But...that the function would be "fooled" becomes increasingly unlikely with each check.

And again, I would like to state that this issue is not about environment detection. The original background example I used was merely illustrative and to state that this change may have (and has) consequences.

At this point, I regret using my original example, as this continually gets more attention both here and in #9279 than that an unintended/unplanned/unannounced breaking change in Node behavior was introduced due to changes in V8.

[ljharb]

fwiw, the only truly reliable means to obtain the global object at the moment - in every version of every JS engine since the beginning of time - is Function('return this')().

To do environment detection could include comparing that object to global, comparing require to that objects' .require, etc.

[kgryte]

@ljharb I know. I left that out of the above code snippet intentionally. The code is not meant to be authoritative, but illustrative.

But a final time, the issue is that a change occurred. People may have come to depend on said change. I asked for clarification on whether this change was intentional and permanent. Based on participant reaction, the change was unexpected. This change, irrespective of how people used the feature '[object global]', may break people's code.

The question that remains is: will Node ensure consistent behavior across all versions (including current LTS versions 4 and 6) or will Node decide to include this as a breaking change going forward.

[sam-github]

Seems better to me to backdate it as a breaking change, and follow v8's lead here. It broke across a major version, breaks are allowed, and node didn't document every detail of how v8 was different and possibly incompatible. Its true it wasn't intended specifically, but it was intended to update v8.

Only @addaleax has suggested a possible fix, and it looked a bit questionable to me - sometime next year we are going to get another bug report asking why node is overriding the base v8 behaviour, and we'll be "well, it was like that in v0.10, and we decided to stay that way forever, even after v8 moved on" and that's not so compelling.

[kgryte]

"well, it was like that in v0.10, and we decided to stay that way forever, even after v8 moved on"

@sam-github Yes, breaks are allowed in major updates. No one is disputing this. However, the break should be considered, not within the context of v0.10, but within the context of current LTS versions. If a break is deemed necessary, then fine. Otherwise, the patch by @addaleax works.

In general, if Node decides to accept a change introduced by V8, then would be nice to have a rationale why Node decided to follow V8's lead. If to ensure consistency and stability across the current Node ecosystem, including LTS releases, Node must break with/patch V8 at certain times, seems reasonable to believe such departures are permissible.

I have yet to see a compelling reason why Node should forgo stability in this case. Unless V8 has a good reason to introduce this change, seems arbitrary and without rationale and introduces maintenance overhead for code currently using this feature.

[Fishrock123]

Been thinking, and I think this should be acceptable within the constraints that we already consider bumping v8 like this as a major.

I'd like to hear exactly how much this effected, how widespread it is. I think if a lot of people are running into this then we should consider rolling it back but if only a few are, we should probably just document the change and accept it.

[addaleax]

fwiw, I tried running a npm grep for \bobject global\b, with these results: https://gist.github.com/addaleax/01e287d8a1674eaeaaa3af725f7653ef

[ljharb]

@addaleax thanks - now we have a list of which packages need PRs/issues, so they can stop checking for the global object the wrong way :-)

[kgryte]

@ljharb I do not believe that is the lesson to draw here. The takeaway is that people have used the fact that, for all Node versions prior to 7, when provided global , Object#toString() returns '[object global]'. And while you could file issues and make PRs on public packages admonishing them for doing the "wrong" thing, you cannot do that for all the private packages and applications which will break due to the removal of this feature.

[hashseed]

I think this is the change that caused this:
https://codereview.chromium.org/2080243003

V8 is following the ES6 spec here, which does not leave much room for interpretation:
https://tc39.github.io/ecma262/#sec-object.prototype.tostring

In d8, we indeed define the toStringTag for the global object to be "global". In Chrome, window[Symbol.toStringTag] is set to "Window". Maybe this is the path node should take as well?

[sam-github]

@hashseed Which path do you suggest node take, setting it to global, or setting it to Window?

[hashseed]

To global to mimic the old behavior.

[dnalborczyk]

This proposals https://github.com/tc39/proposal-global (stage 3) intention is to unify the 'global' object independent of the runtime environment. The name for such an object is 'global' (the same as node.js is currently using, as supposed to self, System.global etc.). Therefore, I suspect that if the proposal is being accepted, and browsers implement it, global.toString() will likely return - according to the spec - '[object Object]' as well.

If we revert this now to mimic pre-v7-behavior, we might need to revisit it again in the future to revert the reverted, if we want consistency between environments.

@ljharb Is that assumption correct?

[ljharb]

The JS spec will continue to not contain or require (nor prohibit, it must be noted) that global have a Symbol.toStringTag defined.

So, despite my belief that there should be no such behavior, I must still acknowledge that there's no spec compliance issue here, either way. Browsers may and will continue to return [object Window], and node can choose to return whatever it wants.

I also don't think there's a compatibility issue here either way, because with the advent of ES6 and Symbol.toStringTag, nobody should ever be relying on the output of Object.prototype.toString ever again for anything but debugging, and to do so is to make your code brittle.

[dnalborczyk]

Thanks @ljharb

I should have been more specific, I meant the spec regarding https://tc39.github.io/ecma262/#sec-object.prototype.tostring referred above.

[ljharb]

@dnalborczyk that spec says that if global[Symbol.toStringTag] === 'global', that Object.prototype.toString.call(global) should return '[object global]'

[kgryte]

@ljharb Never using Object#toString is an unrealistic expectation. Far too much code which already exists in the wild uses Object#toString to check for a whole variety of things, not just '[object global]'. And this is not going to be fixed or stopped. That this would lead to "brittle" code should have been (and probably was) considered during the specification phase.

My understanding is that one motivation, and intended byproduct, of Symbol.toStringTag was to allow people to customize the output of Object#toString and, thus, enable another means of identifying user defined classes. For example,

if ( Object.prototype.toString.call( foo ) === '[object MyVerySpecialClass]' ) {
  // I've identified an instance of MyVerySpecialClass!
} else {
  // Not an instance of MyVerySpecialClass
}

That this feature can also be used for ill is not necessarily a reason to avoid using the behavior. And people will use this behavior for identifying their own custom class instances, just as Chrome sets window[Symbol.toStringTag] = "Window". That Node may want to "customize" its own environmental global seems a realistic possibility.

Truth is, similar to modifying a built-in prototype (that you can do so, by your definition, would make using prototypical methods "brittle" and to be avoided whenever possible, including for builtins!) modifying a foreign object's toStringTag should not be encouraged. But...this should not stop those who define the environment or introduce their own objects from customizing the output of Object#toString, particularly if it means easier identification of said objects, or, in this case, maintaining backward compatibility.

[hashseed]

I think we can safely assume that pre-ES6 code that assumes String(global)=="[object global]" won't interfere with toStringTag, since the latter is also an ES6 feature.

[ljharb]

@kgryte none the less, it's the reality since ES6 erased the unforgeability of Object#toString.

Certainly the case you're describing - as a publicly available opt-in, is a fine use of it. But using it as a brand check is no longer reliable, no matter how much code is doing it.

@hashseed except that in newer environments, global = { [Symbol.toStringTag]: 'global' }; can make that code go down unintended paths.

[hashseed]

Sure. We would have to weigh cases that rely on the old behavior against ones that use toStringTag for other purposes.

[trusktr]

Just noting something curious, but I'm in Node 8 and some functions that I create with new Function appear as [object global] in the console, and .bind() method is missing, which is strange.

Look at the strange output I see when debugging:

Not sure how to reproduce, otherwise I'll open a new issue. Made an issue with repro: #19651