Fix babel helpers to not require `Symbol` to exist #8947

sharmilajesupaul · 2018-10-31T06:37:36Z

Ensure that Symbol exists and is a function before using it.

In the _iterableToArray helper, we update the condition to check for objects that act as arrays such as, objects with the length property, Maps, and Sets. This change is being made because we ran into an issue with the usage of Symbol breaking on ie11 because it is not supported.

Q	A
Fixed Issues?	Fixes #7597
Patch: Bug Fix?	✅
Major: Breaking Change?	❌
Minor: New Feature?	❌
Tests Added + Pass?	✅
Documentation PR Link	N/A
Any Dependency Changes?	❌
License	MIT

cc. @ljharb @loganfsmyth

babel-bot · 2018-10-31T06:41:30Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9378/

ljharb · 2018-10-31T06:46:20Z

packages/babel-runtime/helpers/esm/iterableToArray.js

+    || (iter && 'length' in iter)
+    || (typeof Map !== 'undefined' && iter instanceof Map)
+    || (typeof Set !== 'undefined' && iter instanceof Set)


es6-shim provides both Map and Set but doesn’t provide Symbol.iterator when native Symbols aren’t supported; so these lines are needed.

packages/babel-runtime/helpers/esm/iterableToArray.js

packages/babel-runtime-corejs2/helpers/esm/iterableToArray.js

ljharb · 2018-11-02T20:10:19Z

packages/babel-runtime-corejs2/helpers/esm/iterableToArray.js

 import _isIterable from "../../core-js/is-iterable";
+import _Symbol from "../../core-js/symbol";


This also shouldn't need to be pulled in; it's referring to the global.

It looks like checking the typeof Symbol === 'function' results in the build bringing in import, however, the use of Symbol.iterator in Object(iter) relies on the global. Do we want to explicitly rely on the global in the type check? i.e. typeof global.Symbol === 'function'

Symbol.iterator in something is a special case which transpiles to isIterable helper and it does not load full Symbol polyfill.

nicolo-ribaudo · 2018-11-02T21:26:08Z

For now we could check for Map/Set using toString (like we do for arguments) so that they are not pulled in. I think that, until there is a way to avoid it, importing Symbol is an acceptable compromise for fixing the issue.

ljharb · 2018-11-02T21:56:30Z

@nicolo-ribaudo i think using global.Map might bypass the runtime transform and work appropriately.

packages/babel-helpers/src/helpers.js

zloirock · 2018-11-02T22:14:57Z

importing Symbol is an acceptable compromise for fixing the issue.

Anyway, it's enough heavy dependency and it would be better somehow prevent unnecessary adding of this polyfill.

loganfsmyth · 2018-11-02T23:49:13Z

packages/babel-helpers/src/helpers.js

+      || typeof iter === 'string'
+      || (typeof Symbol === 'function' && Symbol.iterator in Object(iter))
+      || (iter && 'length' in iter)
+      || Object.prototype.toString.call(iter) === "[object Map]"


I'm not sure I think including these is worth it. If it's doesn't pass Array.isArray(iter), then we can ask people to load a symbol polyfill or convert it to an array before using it as an iterable, since it really isn't one unless they've done one of those two things anyway.

es6-shim creates a Map and Set that work with Array.from, without Symbol.iterator - without this change, Airbnb remains broken on environments without symbols.

My issue here is that i'd consider the use of Array.from an implementation detail. If Symbol doesn't exist, then your item isn't iterable. The fact that we support it for arrays is a necessity, but I don't see it that way for Map and Set. I'd expect you to call Array.from to convert to an array first.

Anyway, now it will not work with es6-shim because it does not support @@toStringTag.

I don’t think it is an implementation detail.

In an environment with Symbols, i agree - either it’s iterable or not. But in an environment without symbols, there’s no spec to govern it - and it’s entirely reasonable to say that Array.from can accept any builtin that should be iterable - arrays, strings, Sets, and Maps.

This worked fine in babel 6. I’m not sure why it needs to be dropped in this part of babel 7.

@zloirock is correct that toString won’t suffice. It needs to use instanceof on the global Map/Set to be able to detect this case.

@loganfsmyth Would you consider two branches - one where Symbols exist, which only checks Symbol.iterator, and another where they don’t, that has the (zero-dependency) fallback logic we’ve detailed here?

zloirock · 2018-11-03T07:09:17Z

In babel@6 we used some dirty hacks for preventing transpiling some built-ins by runtime. Maybe makes sense add something like directives for that? Something like // babel-runtume-disable-next-line?

nicolo-ribaudo · 2018-11-04T17:59:39Z

@sharmilajesupaul I just noticed that I merged 38397ce which uses Symbol without checking if it exists (in the toPrimitive helper) 😅 Could you please add it in this PR?

sharmilajesupaul · 2018-11-04T19:36:31Z

@nicolo-ribaudo yup, i can update it here, will get to PR again this afternoon.

Update babel helpers to not require Symbol to exist. We ran into an issue with ie11 where Symbol is not supported. This PR gaurds the usage of Symbol by ensuring that it exists first. There are also changes that allow objects with a length property and array-like objects to work with `iterableToArray`.

skliffblizz · 2018-11-07T16:40:09Z

This fix is most important to my organization right now. Can't transition to latest babel version until IE11 works.

clshortfuse · 2019-03-30T13:59:10Z

@zloirock If that's indeed the new direction of Babel, then somebody needs to update

https://babeljs.io/docs/en/caveats

And change it to reflect that Spread now requires Symbol.

clshortfuse · 2019-03-30T16:07:42Z

I made PR #9794 that only targets Spread support so it can work better on IE11 (after you polyfill Array.from). It's less ambitious and can at least solve the v7 regression.

nicolo-ribaudo · 2019-03-30T21:19:57Z

I have added this PR to the next meeting agenda.

ljharb · 2019-03-30T21:23:29Z

It's a bad idea to attempt faking any kind of Symbol support ("spec" mode shouldn't work unless typeof Symbol.iterator === 'symbol', if you want to be that pedantic about it), but that debate doesn't need to be had neither here nor at all.

Both models should be supported (shamming Symbol, or not), and this is both an undocumented/unintended regression from babel 6 to 7, and also one that makes babel strictly less useful by forcing use of a Symbol polyfill where it need not.

zloirock · 2019-03-30T23:34:52Z

It's a bad idea to attempt faking any kind of Symbol support

It's the base of a big part of babel features for a long time and the practice shown that it a good idea.

"spec" mode shouldn't work unless typeof Symbol.iterator === 'symbol', if you want to be that pedantic about it

It's an absolutely strange requirement since well-known symbol definition is another part of the spec. However, it could work with babel / core-js anyway because we have typeof transform.

Both models should be supported

I agree. But adding special cases and dirty hacks only for Array.from polyfill without Symbol support, but with the support of some iterables, to the standard mode is a bad idea - more other, it can be implemented in loose mode without those special cases and hacks.

ljharb · 2019-03-31T04:57:50Z

The problem is that loose mode enables other undesired consequences. Again, the spec does not govern an environment with Array.from or spread but without Symbols - as such, babel gets to make a choice here - between "it works intuitively in these environments" or "it breaks in these environments".

mattpilott · 2019-04-03T16:14:51Z

Has v3 resolved this now?

ljharb · 2019-04-03T22:17:02Z

@matt3224 no, core-js can't possibly solve it because the PR is about fixing babel when core-js is not included (specifically, when a Symbol sham is not applied).

zloirock · 2019-04-09T16:37:44Z

@ljharb could you add support of @@toStringTag behavior for Map and Set to es6-shim (even without exposing Symbol.toStringTag)? It could seriously simplify of fixing this issue.

ljharb · 2019-04-09T16:59:01Z

Only in an environment where native symbols exist; this issue won’t be helped by that.

zloirock · 2019-04-09T20:23:32Z

If Object.prototype.toString.call(new Map) will be [object Map] with es6-shim, this will simplify the issue as much as possible.

ljharb · 2019-04-09T20:55:31Z

es6-shim would use a cached toString, so for a non-symbol environment, it'd only solve it when core-js had been run prior to es6-shim - the whole point is that babel should work without core-js.

zloirock · 2019-04-09T21:12:28Z

@ljharb how it's related core-js? This helper works with core-js without any dirty hacks. One more time: I meant adding of internal @@toStringTag for Object.prototype.toString and Map / Set behavior without exposing Symbol.toStringTag like es6-shim have an internal @@iterator without exposing Symbol.iterator. Polyfill of Object.prototype.toString by ES6 spec in es6-shim.

ljharb · 2019-04-09T21:16:06Z

Ah, I see what you mean. That's certainly an option; but since in a world with Symbols, Object.prototype.toString isn't reliable, I'd hope babel isn't relying on that result for anything.

zloirock · 2019-04-09T21:19:41Z

instanceof is not reliable too. And this helper is not 100% meet the specification anyway.

MatthewHerbst · 2019-07-17T01:23:30Z

I have added this PR to the next meeting agenda.

@nicolo-ribaudo what was the result of the meeting? (Or has it not happened yet?)

johnnywang · 2019-07-19T22:52:57Z

packages/babel-helpers/src/helpers.js

+      || typeof iter === 'string'
+      || (typeof Symbol === 'function' && Symbol.iterator in Object(iter))
+      || (iter && 'length' in iter)
+      || Object.prototype.toString.call(iter) === "[object Arguments]"
    ) return Array.from(iter);


So I encountered the originating problem on IE11 in the JS library I maintain as well, because we can't use babel-polyfill either for general compatibility. Wasn't sure how long this PR would take to go through so I tried seeing what would happen if we just polyfill Symbol, and that surfaced up this line as another issue: Array.from is also not supported in IE11.

So if you're encountering the Symbol problem in IE11, it's probably because you aren't using polyfills, which means this will likely be an issue as well. Not sure if that's tangential enough to be in a separate ticket entirely though.

using es6-shim, Array.from works but symbols don’t, leading to this issue.

It’s reasonable to require shimming Array.from; but not to require shamming symbols.

peter17 · 2019-09-04T07:55:01Z

Sorry to disturb, but this PR seems stuck... Can someone sum up why? We really need #7597 fixed... Thanks in advance.

esprehn · 2019-09-12T19:38:09Z

Hey @zloirock what would be the set of changes you would want to see in this PR to get it merged? We're maintaining a forked copy of the babelHelpers internally at Airbnb to work around this, but that's pretty unfortunate. It also seems other folks are also hitting this issue.

I'd love to see this get unstuck, let us know how we can help!

zloirock · 2019-09-13T07:26:17Z

@esprehn I already wrote it many times - it should not inject unnecessary polyfills in runtime and preset-env and should not break the existent model of injecting polyfills (like in the current version of this PR).

zloirock · 2019-09-13T07:29:39Z

With the current version of this PR, if the library contains code like

if (typeof Symbol == 'undefined') throw Error('This library requires `Symbol` polyfill');

it will die at this line even with runtime.

sharmilajesupaul · 2020-02-17T19:26:57Z

closing due to inactivity, feel free to reopen or make another PR for this issue to push this forward

ljharb approved these changes Oct 31, 2018

View reviewed changes

ljharb requested review from loganfsmyth, jridgewell and nicolo-ribaudo October 31, 2018 06:47

ljharb added the 7.x: regression label Oct 31, 2018

sharmilajesupaul force-pushed the gaurd-against-symbol-usage branch 2 times, most recently from f2522e4 to 9f84c1b Compare October 31, 2018 06:55

ljharb changed the title ~~Fixes #7597~~ Fix babel helpers to not require Symbol to exist Oct 31, 2018

nicolo-ribaudo reviewed Oct 31, 2018

View reviewed changes

packages/babel-runtime/helpers/esm/iterableToArray.js Show resolved Hide resolved

existentialism added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Oct 31, 2018

sharmilajesupaul force-pushed the gaurd-against-symbol-usage branch 2 times, most recently from 6a1ebbb to 3ad4dfe Compare November 2, 2018 01:39

ljharb approved these changes Nov 2, 2018

View reviewed changes

nicolo-ribaudo approved these changes Nov 2, 2018

View reviewed changes

zloirock requested changes Nov 2, 2018

View reviewed changes

packages/babel-runtime-corejs2/helpers/esm/iterableToArray.js Outdated Show resolved Hide resolved

ljharb requested changes Nov 2, 2018

View reviewed changes

zloirock reviewed Nov 2, 2018

View reviewed changes

packages/babel-helpers/src/helpers.js Show resolved Hide resolved

loganfsmyth reviewed Nov 2, 2018

View reviewed changes

sharmilajesupaul added 3 commits November 5, 2018 09:10

Use toString to check if param is a Map or Set

54d7b49

Check for Symbol in toPrimitive

6785ec9

sharmilajesupaul force-pushed the gaurd-against-symbol-usage branch from ca35f9a to 6785ec9 Compare November 5, 2018 17:59

nicolo-ribaudo mentioned this pull request Mar 30, 2019

Babel meeting - 9th April babel/notes#90

Closed

3 tasks

lencioni mentioned this pull request Jun 17, 2019

Use @babel/plugin-transform-runtime for helpers airbnb/babel-preset-airbnb#51

Merged

MatthewHerbst mentioned this pull request Jul 17, 2019

'Symbol' is undefined problem in ie11 #7597

Closed

johnnywang reviewed Jul 19, 2019

View reviewed changes

eps1lon mentioned this pull request Nov 1, 2019

fix: Remove Symbol usage for older environments A11yance/aria-query#27

Merged

saydmateen mentioned this pull request Dec 5, 2019

Tree breaking in IE 11: 'Symbol' is undefined deltoss/d3-mitch-tree#16

Closed

sharmilajesupaul closed this Feb 17, 2020

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 19, 2020

github-actions bot locked as resolved and limited conversation to collaborators May 19, 2020

		import _isIterable from "../../core-js/is-iterable";
		import _Symbol from "../../core-js/symbol";

Fix babel helpers to not require Symbol to exist #8947

Fix babel helpers to not require Symbol to exist #8947

Conversation

sharmilajesupaul commented Oct 31, 2018 • edited

babel-bot commented Oct 31, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zloirock Nov 2, 2018 • edited

Choose a reason for hiding this comment

nicolo-ribaudo commented Nov 2, 2018

ljharb commented Nov 2, 2018

zloirock commented Nov 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb Nov 3, 2018 • edited

Choose a reason for hiding this comment

zloirock commented Nov 3, 2018 • edited

nicolo-ribaudo commented Nov 4, 2018

sharmilajesupaul commented Nov 4, 2018

skliffblizz commented Nov 7, 2018

clshortfuse commented Mar 30, 2019 • edited

clshortfuse commented Mar 30, 2019

nicolo-ribaudo commented Mar 30, 2019

ljharb commented Mar 30, 2019 • edited

zloirock commented Mar 30, 2019

ljharb commented Mar 31, 2019

mattpilott commented Apr 3, 2019

ljharb commented Apr 3, 2019

zloirock commented Apr 9, 2019

ljharb commented Apr 9, 2019

zloirock commented Apr 9, 2019

ljharb commented Apr 9, 2019

zloirock commented Apr 9, 2019 • edited

ljharb commented Apr 9, 2019

zloirock commented Apr 9, 2019

MatthewHerbst commented Jul 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peter17 commented Sep 4, 2019

esprehn commented Sep 12, 2019

zloirock commented Sep 13, 2019 • edited

zloirock commented Sep 13, 2019

sharmilajesupaul commented Feb 17, 2020 • edited

Fix babel helpers to not require `Symbol` to exist #8947

Fix babel helpers to not require `Symbol` to exist #8947

sharmilajesupaul commented Oct 31, 2018 •

edited

babel-bot commented Oct 31, 2018 •

edited

zloirock Nov 2, 2018 •

edited

ljharb Nov 3, 2018 •

edited

zloirock commented Nov 3, 2018 •

edited

clshortfuse commented Mar 30, 2019 •

edited

ljharb commented Mar 30, 2019 •

edited

zloirock commented Apr 9, 2019 •

edited

zloirock commented Sep 13, 2019 •

edited

sharmilajesupaul commented Feb 17, 2020 •

edited