Skip to content
This repository was archived by the owner on Dec 7, 2021. It is now read-only.

Upgrade babili preset to new package name#847

Merged
usergenic merged 3 commits intomasterfrom
upgrade-babili-minify
Sep 25, 2017
Merged

Upgrade babili preset to new package name#847
usergenic merged 3 commits intomasterfrom
upgrade-babili-minify

Conversation

@TimvdLippe
Copy link
Copy Markdown
Contributor

@TimvdLippe TimvdLippe commented Aug 15, 2017

Installing the polymer-cli showed the following message:

babili has been renamed to babel-minify. Please update to babel-preset-minifybabel-preset-minify

This PR upgrades babel-preset-babili to the new name and fix breaking changes from 0.1.0. The unsafe option was removed and I updated the options to reflect the equivalent behavior. One change that was apparent was the removal of parenthesis from arrow functions (see the changed test). I am not sure if this is a bug or change in behavior of babili.

  • CHANGELOG.md has been updated

@TimvdLippe
Copy link
Copy Markdown
Contributor Author

Ping @FredKSchott This should make sure that minification works with ES6 and Safari. I hit that bug by using <script type="module"> in production.

@justinfagnani
Copy link
Copy Markdown
Contributor

@TimvdLippe can you rebase?

@justinfagnani justinfagnani self-requested a review September 18, 2017 21:37
@TimvdLippe TimvdLippe force-pushed the upgrade-babili-minify branch from 93b4056 to 522391f Compare September 18, 2017 21:59
@TimvdLippe
Copy link
Copy Markdown
Contributor Author

@justinfagnani Done 👍

@web-padawan web-padawan mentioned this pull request Sep 25, 2017
1 task
[minifyPreset(null, {
flipComparisons: true,
guards: true,
typeConstructors: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@TimvdLippe are these settings equivalent to the simplifyComparisons setting or are there any meaningful differences we should note in CHANGELOG etc?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, maybe a plugin has to be included for that now: https://github.com/babel/minify/tree/master/packages/babel-plugin-transform-simplify-comparison-operators, but given that the plugin was not supposed to run, not including it has the same meaning? Not 100% on that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/derp I just noticed your description mentioning the paren-removal change identified by tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like simplifyComparisons was hoisted out of unsafe into top level of options. I'm able to get the tests to pass with the following...

   constructor() {
     super({
       presets:
          [minifyPreset(null, {simplifyComparisons: false})]
     });
   }

Was that missed or do these options matter in other ways not covered:

            flipComparisons: true,
            guards: true,
            typeConstructors: true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm that is really weird, I couldn't find it in the options. If that fixes it, then yes please! Feel free to commit to this branch 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok will push

Copy link
Copy Markdown
Contributor

@usergenic usergenic left a comment

Choose a reason for hiding this comment

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

Add note to CHANGELOG- even if non-functional change we should note the package change there.

@TimvdLippe
Copy link
Copy Markdown
Contributor Author

@usergenic done!

@usergenic usergenic merged commit ebf8648 into master Sep 25, 2017
@usergenic usergenic deleted the upgrade-babili-minify branch September 25, 2017 20:19
@jogibear9988
Copy link
Copy Markdown

as this change does not fix the problem with the broken js, could #878 be merged?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants