Symbol.iterator polyfill in src/new-tab.js breaks Symbol in IE11

[natanaelsinisalo]

Hi @markjaquith and thanks for the great work on this plugin 🙂

I just spent a few hours on squishing a weird IE11 bug that seems to be caused by this plugin. And more specifically the Symbol.iterator polyfill found in the src/new-tab.js file.

By loading only the iterator polyfill (import 'core-js/features/symbol/iterator';) without the Symbol polyfill (import 'core-js/features/symbol';) the Symbol.iterator polyfill makes typeof Symbol "object" in IE11 instead of "function" that it should be. You can find the exact problem that is causing it here.

I made some testing and the quickest way to fix this is by adding import 'core-js/features/symbol'; in the src/new-tab.js file before the import 'core-js/features/symbol/iterator';.

It would also be relevant to consider if bundling any polyfills into a WordPress plugin is a good thing. I understand that most of the people doesn't have a setup where they can add polyfills into their WordPress installation when they need it, but polyfills can quickly start causing more problems (by conflicting with each other) than they solve if everybody starts adding polyfills into their plugins.

There are some good (although a few years old) discussion on this topic here and here.

Please note that I'm not trying to push any puristic agenda on this. Just wanted to give you some perspective in case you haven't run into this kind of issues before 🙈

[markjaquith]

@natanaelsinisalo Thank you so much for your thoughtful note. I burned a ton of time sorting out IE issues, and it probably shouldn't be surprising that my best efforts here yielded downstream woes (ugh, I feel your pain).

I think I'll take your fix for now, but yeah, I definitely agree that bundling polyfills isn't a great solution. Ironically, the whole reason I'm in this mess is because I rewrote the frontend JS to not use jQuery because I got tired of crappy WordPress themes hardcoding their own old-and-busted version of jQuery (and frankly, jQuery is too big a dependency). And now I'm being "that guy" whose code is bleeding out and affecting other code.

I probably have two long terms solutions here:

1. Remove IE support (use IE conditional tags to not run my JS on IE).
2. Rewrite the code to not use Symbols or el.closest() or any of the nice things that make writing JavaScript bearable in The Year of Our Lord Two Thousand And One Score.

I know which one I should probably do. 😭

[natanaelsinisalo]

Totally forgot to reply you back 🙈

If my opinion weights anything, I'd just remove the IE support altogether. I know Microsoft is still supporting IE11 until the end of 2025, but we as a developers can play our part of killing it earlier by just not supporting it any more. Like YouTube did with IE6.

But I understand if you choose to rewrite the code to not use symbols to preserve the IE support. WordPress community is famous for supporting legacy stuff and that's completely fine too.

How about implementing the solution I recommended earlier? Any estimate on that? I could do a pull request, but since it's just one line of code, it would probably be faster for you to add it and make sure everything works after that. I already tested it in my local environment and it indeed solved the issue without any noticeable side effects.

[markjaquith]

If you send a PR I’ll get it handled today.

[natanaelsinisalo]

Sure, and sorry for the delayed reply again. Had a few really busy weeks.

I've now made a pull request (#110) to solve this issue.

[markjaquith]

Finally got this merged in a bug scrub. Will be in the next release. Thanks again!

[natanaelsinisalo]

You're welcome! :)