Skip to content
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(gatsby): Prevent minifier from compressing output using ES6+ syntax #9135

Merged
merged 1 commit into from
Oct 16, 2018
Merged

fix(gatsby): Prevent minifier from compressing output using ES6+ syntax #9135

merged 1 commit into from
Oct 16, 2018

Conversation

shanekenney
Copy link
Contributor

@shanekenney shanekenney commented Oct 15, 2018

Fixes #8905
Fixes #7003

The previous configuration allowed terser to take opportunities to compress the output code using ES6+ syntax. This PR configures terser to parse ES8 but prevents any ES6+ optimizations in the output.

from https://github.com/terser-js/terser#output-options

The ecma option will only change the output in direct control of the beautifier. Non-compatible features in the abstract syntax tree will still be output as is. For example: an ecma setting of 5 will not convert ES6+ code to ES5.

@shanekenney shanekenney requested a review from a team as a code owner October 15, 2018 23:26
@DSchau DSchau changed the title fix: Prevent minifier from compressing output using ES6+ syntax fix(gatsby): Prevent minifier from compressing output using ES6+ syntax Oct 16, 2018
@pieh
Copy link
Contributor

pieh commented Oct 16, 2018

Was just testing it locally and seems to be working fine, here's example of how output is changed:

diff public/component---src-pages-index-js-7dbbc3a98b9625b0e3a9.js public-master/component---src-pages-index-js-7dbbc3a98b9625b0e3a9.js
40c40
<             return o.a.createElement(s, Object.assign({ t: t }, e))
---
>             return o.a.createElement(s, Object.assign({ t }, e))

so with changes in this PR it produce correct output and before it didn't and it was breaking IE11 for sure

---edit
Note I formatted output code, so diff is actually readable :) It is still minified of course

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @shanekenney for PR and @simonjoom for raising issue and both of you for providing more context! 👍

@pieh pieh merged commit fed6d35 into gatsbyjs:master Oct 16, 2018
@gatsbot
Copy link

gatsbot bot commented Oct 16, 2018

Holy buckets, @shanekenney — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (Currently we’ve got a couple t-shirts available, plus some socks that are really razzing our berries right now.)
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

@pieh
Copy link
Contributor

pieh commented Oct 16, 2018

Published as gatsby@2.0.25

jedrichards pushed a commit to jedrichards/gatsby that referenced this pull request Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants