Skip to content

Update terser to v5, support async minifiers - #606

Closed
janicduplessis wants to merge 1 commit into
react:mainfrom
janicduplessis:update-terser
Closed

janicduplessis wants to merge 1 commit into
react:mainfrom
janicduplessis:update-terser

Conversation

@janicduplessis

Copy link
Copy Markdown
Contributor

Summary

Update terser to the latest version that supports more modern js syntax like optional chaining. The main breaking change in v5 is that minify is now async. To support this I also needed to add async minifier support in metro-transform-worker.

Test plan

  • Run tests
  • Test building my app bundle with minifier terser and untransformed optional chaining

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 7, 2020
* comment and run Flow. */
const result = terser.minify(code, options);

if (result.error) {

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.

Terser will now throw an error so there is no need for additional handling anymore.

@cpojer cpojer left a comment

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.

Thank you 👍

@facebook-github-bot facebook-github-bot left a comment

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.

@cpojer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@cpojer

cpojer commented Nov 11, 2020

Copy link
Copy Markdown
Contributor

We have a big integration with Metro and Buck internally that relies on this code being sync and changing every Flow to be async is a ton of work I unfortunately won't be able to get to.

The good news is that with Hermes we shouldn't need a minifier, so we may be able to clean up our internal integration soon.

@janicduplessis

janicduplessis commented Nov 11, 2020 •

Copy link
Copy Markdown
Contributor Author

I see, so you are using the metro-minify-terser package directly? What about landing the changes in metro-transform-worker only so it supports async minifiers? This would make it easy then to just publish a terser-v5 minify plugin.

@cpojer

cpojer commented Nov 11, 2020

Copy link
Copy Markdown
Contributor

No, we don't actually use terser but we use the API synchronously in metro-transform-worker pretty heavily deep down a stack where changing everything to be async would be quite a bit of work. As said, with Hermes we may not need to support minifiers for much longer so maybe we can clean that up internally and then merge this.

@janicduplessis

Copy link
Copy Markdown
Contributor Author

Ah ok, the function in transform-worker is already async, but I guess it works since there are no await in it currently. Alright in this case let's just keep this open and we can revisit later when the internal infra changes.

@rockwotj

rockwotj commented Jan 4, 2022

Copy link
Copy Markdown
Contributor

Is there any hope to merging this? Our app is using terser and we've ran into a couple of terser bugs that it impossible to minify our app

@robhogan

Copy link
Copy Markdown
Collaborator

FYI, I’ve rebased this internally now that the groundwork @cpojer mentioned has been done behind the scenes of #754, hope to land it shortly.

nevilm-lt pushed a commit to nevilm-lt/metro that referenced this pull request Mar 14, 2022
Summary:
**Summary**

Update terser to the latest version that supports more modern js syntax like optional chaining. The main breaking change in v5 is that minify is now async. To support this I also needed to add async minifier support in metro-transform-worker.

**Test plan**

- Run tests
- Test building my app bundle with minifier terser and untransformed optional chaining

Pull Request resolved: react#606

Reviewed By: feedthejim

Differential Revision: D24883529

Pulled By: rh389

fbshipit-source-id: c9ed893535337ecc1ce16e433e3d689c2037be0a
nevilm-lt pushed a commit to nevilm-lt/metro that referenced this pull request Apr 21, 2022
Summary:
**Summary**

Update terser to the latest version that supports more modern js syntax like optional chaining. The main breaking change in v5 is that minify is now async. To support this I also needed to add async minifier support in metro-transform-worker.

**Test plan**

- Run tests
- Test building my app bundle with minifier terser and untransformed optional chaining

Pull Request resolved: react#606

Reviewed By: feedthejim

Differential Revision: D24883529

Pulled By: rh389

fbshipit-source-id: c9ed893535337ecc1ce16e433e3d689c2037be0a
nevilm-lt pushed a commit to nevilm-lt/metro that referenced this pull request Apr 22, 2022
Summary:
**Summary**

Update terser to the latest version that supports more modern js syntax like optional chaining. The main breaking change in v5 is that minify is now async. To support this I also needed to add async minifier support in metro-transform-worker.

**Test plan**

- Run tests
- Test building my app bundle with minifier terser and untransformed optional chaining

Pull Request resolved: react#606

Reviewed By: feedthejim

Differential Revision: D24883529

Pulled By: rh389

fbshipit-source-id: c9ed893535337ecc1ce16e433e3d689c2037be0a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants