Fix typeof Symbol.prototype

[brainlock]

fix typeof Symbol.prototype

Babel uses a helper function to return the correct value for typeof obj when obj is a Symbol and support for Symbol has been polyfilled.

This function assumes that obj.constructor === Symbol implies typeof obj === 'symbol'.

This isn't true when obj is Symbol.prototype. In that case (REPL from
node 6, the same holds in Firefox):

> Symbol.prototype.constructor === Symbol
true
> typeof Symbol.prototype
'object'
>

AFAICS, that's the only case where the assumption doesn't hold.

The test added by this patch fails only on node 0.10, as 0.12 already
has a native implementation of Symbol and the polyfill code doesn't run.

This caused a problem in core-js when it's compiled with babel (the
issue was isolated by @skozin here:
zloirock/core-js#189 (comment)).

[codecov-io]

Current coverage is 88.28% (diff: 100%)

No coverage report found for master at b871a49.

Powered by Codecov. Last update b871a49...c93dffb

[danez]

I just tested it in chrome and the babel repl.
👍 lgtm
Thank you very much for contributing your first PR. 🎉

[jridgewell]

This'll also happen anywhere we test typeof on an object that points to Symbol.protoype:

var obj = Object.create(Symbol.prototype);
console.log(typeof obj); // => "symbol"

[brainlock]

@danez

Thank you very much for contributing your first PR. 🎉

Thanks! I'm happy to contribute, we rely a lot on babel.

@jridgewell

Good point. We would in theory be able to polyfill typeof correctly with one additional check:

... && !Symbol.prototype.isPrototypeOf(obj)

(or the equivalent instanceof).

This would be possible if the runtime polyfills for Symbol (e.g. the one provided by core-js) could be implemented in such a way that

Symbol.prototype.isPrototypeOf(Symbol())
> false
Symbol() instanceof Symbol 
> false

for a polyfilled Symbol.

To do that, we could split the exposed global Symbol and Symbol.prototype from the internal ones used to create the instances (make copies, so that === checks fail): this would fix the instanceof (and equivalent isPrototypeOf) checks above.

The problem with that is that we would still not achieve the following:

Symbol() instanceof Symbol
> false
Object(Symbol()) instanceof Symbol 
> true

We would fix the former, but the latter would then change to return false (in current polyfills, both are true).

So if we do that we'd also have to replace the global Object to recognize a polyfilled Symbol instance and convert it to an object obj for which Symbol.prototype.isPrototypeOf(obj) is true.

Do you think this is even doable safely? Worth it?

If not, we'll have to accept that Symbol cannot be polyfilled completely, and teach people to only use a safe subset of Symbol if their code has to run on polyfilled implementations.

(This PR is then not achieving full correctness, but fixing one case that I have seen used and is currently broken when compiled through babel)

[glebcha]

I think it should be merged urgently.