New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(pretty-format): Crash in ES5 environments #9635
Conversation
Only test I have is using |
Wanted to point out that IE 11 is just a specific environment. It crashes on any environment that only supports ES5. Since this packages has a dedicated build for those environments ( |
Shouldn't we tinker our build config, so it compiles away Object.entries instead? |
That's a possibility. Though you would need to compile node_modules/ in this case. |
We compile (and bundle) for browser, so I think it makes more sense to transpile than downgrade |
I'll take a look at the build setup. Thanks for the feedback! |
Sorry, I was supposed to link this but got distracted: https://github.com/facebook/jest/blob/master/scripts/browserBuild.js. It's the browser build script, theoretically targeting ES5 |
No worries I found it already. We have to keep in mind though that using I don't know if any of the affected packages are used in user-facing bundles. If so then going from 102KB unminified to 537KB is not acceptable. I personally don't care about the size of this package since I only use it when running tests in the browser. Bundle size is no major concern for these cases. |
Codecov Report
@@ Coverage Diff @@
## master #9635 +/- ##
==========================================
+ Coverage 65.09% 65.11% +0.01%
==========================================
Files 287 287
Lines 12144 12144
Branches 3009 3009
==========================================
+ Hits 7905 7907 +2
+ Misses 3604 3603 -1
+ Partials 635 634 -1
Continue to review full report at Codecov.
|
Yikes. Is it possible to target just the offending package? |
I already did. Transform-runtime with core-js is just that overzealous. I can take a closer look at the added bytes and see how a prod bundle would look like. |
We'll be splitting out the es5 builds in Jest 26 (so you'll install |
@SimenB es5 builds are now removed, but we didn't provide them as separate packages just yet. |
Right... We can create those packages, I guess. Telling people to transpile themselves sounds good, but I think being able to use these packages via a script tag is useful |
No longer interested in making this work. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Fixes a crash in ES5 environments (or other environments not implementing Object.entries)
Test plan
Edit: Only test I have is using
prett-format@24
vspretty-format@25
Don't have any (because the dependency is inlined) unless you somehow build canaries that I can install. I know that
ansi-styles@3
does not useObject.entries
whileansi-styles@4
does.