mocha icon indicating copy to clipboard operation
mocha copied to clipboard

do not leak polyfills into global context; closes #4423

Open boneskull opened this issue 3 years ago • 5 comments

when we transitioned to Rollup/Babel, we misconfigured Babel such that all language APIs polyfilled by core-js would be accessible not just by Mocha, but by tests and code under test.

This change attempts to "restrict" the polyfills to Mocha itself.

I'm not entirely sure the best way to test this, since our tests also get bundled and would thus have access to our polyfills. We need to basically add a separate script to a Karma config which would get loaded via <script> tag to make any assertions about the global environment. I am too lazy to do this rn.

Will it work with IE? Let's find out.

Bonus: 50k bundle size reduction!

boneskull avatar Sep 14 '20 23:09 boneskull

Coverage Status

Coverage remained the same at 93.959% when pulling c190c40be88989ab86abdb68e95c34cbf6ca9394 on boneskull/issue/4423-polyfill-leak into 738a57574f3eb82ba2844c8a512cafec974ed5ab on master.

coveralls avatar Sep 14 '20 23:09 coveralls

still misconfigured... i hate javascript

boneskull avatar Sep 14 '20 23:09 boneskull

yeah, I am not sure how to do this. using the "runtime" babel helper, doesn't make sense, because everything must be bundled (no externals). using "bundled" pollutes the globals.

boneskull avatar Sep 15 '20 00:09 boneskull

Ugh. Babel is clearly not built for this. It's possible we should try buble instead. That only handles syntax though. Then we'd have to curate our polyfills ourselves. But it seems preferable to the magic that's keeping us from getting the output we want right now

Munter avatar Sep 18 '20 07:09 Munter

I think I could curate the polyfills w/o needing to swap out babel. it just seems incredibly weird that babel won't handle this for us.

afaict there are some movements in this direction (babel-polyfill-corejs3 for instance) but I haven't been able to get it working. seems experimental at this point, but I think that's the problem trying to be solved.

configuring babel is so much not fun. it is very low-level, and I think our use-case is unique enough to make something higher-level out of reach as wel

boneskull avatar Sep 18 '20 16:09 boneskull

Closing to keep our PR queue clean as there are a lot of merge conflicts. But we'll take another look at the underlying issue when we have time - reducing the size & clobbering-ness of polyfills is going to be a nice thing for us to do.

JoshuaKGoldberg avatar Mar 01 '24 21:03 JoshuaKGoldberg