Closure-compiler: Symbol polyfill returns strings, creating issues for other libraries

[gaearon]

Hi 👋

It seems like GCC ships a low-fidelity Symbol polyfill that returns strings. This has broken numerous libraries, including React, which assumes that if Symbol and Symbol.for exists, the typeof of the returned value is not string (popular polyfills like core-js return objects).

Unfortunately this issue isn't isolated to GCC apps. Some Google SDKs (notably, Google Maps SDK) are compiled with GCC, polluting the environment with this polyfill. This is currently breaking every React website that uses Google Maps API in IE11 (https://github.com/facebook/react/issues/13414).

React assumes that if Symbol and Symbol.for exist, typeof Symbol() will never be a string. Violating this leads to very confusing errors for React users. It's not obvious at all that loading a Maps API will install a low-fidelity polyfill into the global environment.

While arguably both string and object values for typeof are wrong, in practice Symbols most commonly need to be differentiated from strings — because both can serve as keys. So typically the value is tested as typeof something === 'string' to disambiguate them (because checking it for 'symbol' inside a library would only work with native Symbols). Therefore in practice making the typeof value an object creates less footguns — and has been used by popular polyfills like core-js for a few years.

Happy to provide more details if necessary! You can also find a reproducing project (caused by Google Maps being compiled with GCC) here: https://github.com/jamiewinder/ie11-fragment-issue

[lauraharker]

@brad4d can you triage this?

[brad4d]

This is related to #2825

The problem there is that Object.getOwnPropertyNames(o) will return keys that are supposed to be symbols, but are actually strings created by closure-compiler's Symbol polyfill.

[gaearon]

In our case I don't think getOwnPropertyNames() matters in particular. The issue that bites us is that Symbols are polyfilled as strings rather than objects. I guess technically we could adopt our code to check for that first. But it would be nice to align GCC with what more popular polyfills do.

[brad4d]

Some notes:

1. closure-compiler does not provide a polyfill for Symbol.for.
2. closure-compiler's polyfills in general are designed so that if the thing being polyfilled already exists, it is left alone and not replaced.

Given the above, if Symbol.for exists, then our Symbol polyfill should be disabled.
So it should be correct that if Symbol.for exists then Symbol() will always return an actual symbol.

Is it possible that there's another polyfill running on top of ours here?

1. closure-compiler Symbol polyfill runs early.
2. another polyfill library loads later and tries to add Symbol.for to our Symbol.
3. Stuff is broken because the later library assumes it is building on its own Symbol implementation.

[gaearon]

This is likely what happens, yes. (In the reported example the person was running core-js.)

[brad4d]

In that case the problem could be resolved by making sure the library providing the desired Symbol polyfills loads before the Maps API (or any other JS compiled with closure-compiler).
That way the closure-compiler polyfill code will see that Symbol is already defined by the more feature-rich library, so it will be left unmolested.

[gaearon]

Thanks, I'll note that in the issue. That said it's not always easy because typically main polyfills are bundled with app's JS, but app's JS expects any libraries (like Google Maps) to already have been loaded.

[concavelenz]

Returning an object rather than a string seems heavy weight but I can see the value of not return typeof string.

I'm curious though as to where the distinction is actually helpful? Can you provide a reference to the code in question that is trying to distinguish using "typeof someSymbol"?

[jplaisted]

So this is coming up again now that Symbol.prototype.description is proposed.

Is the tl;dr here that we should make our Symbol polyfill an object rather than a string? That is needed for description too. Why wasn't this made as an object before, exactly?

[blickly]

Created internal issue b/123778554

[brad4d]

@jplaisted I think what we need to do here is go ahead and make our Symbol polyfill an Object.
As you said, we need that to implement Symbol.prototype.description anyway.
Could you close this GitHub issue once that's done?

Thanks