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

Allow for async minifiers #754

Closed
wants to merge 1 commit into from
Closed

Allow for async minifiers #754

wants to merge 1 commit into from

Conversation

rockwotj
Copy link
Contributor

@rockwotj rockwotj commented Jan 7, 2022

Summary

Terser v5 is async, so if we want to upgrade to support the latest syntax we need this.

Test plan

Existing unit tests - this only adds an await clause, which is safe to do to non-promises, so it shouldn't break anything.

Terser v5 is async, so if we want to upgrade to support the latest syntax we need this.
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 7, 2022
@rockwotj
Copy link
Contributor Author

rockwotj commented Jan 7, 2022

#606 is related - but this doesn't change the terser package - I'll send another PR for that. That PR was never merged because of a need for using the transform worker sync, but I believe that is obsolete as there is already a bunch of await clauses in that file

@facebook-github-bot
Copy link
Contributor

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

@rockwotj
Copy link
Contributor Author

@sota000 is there any hope of merging this?

@robhogan
Copy link
Contributor

@rockwotj this requires a few modifications to the way we use metro internally, but it’s in the pipeline

@rockwotj
Copy link
Contributor Author

Thanks so much!

@robhogan
Copy link
Contributor

FYI @rockwotj - no need to send another PR for updating to terser v5 - I’ve picked up #606, updated internally and it’s in review now.

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

Terser v5 is async, so if we want to upgrade to support the latest syntax we need this.

**Test plan**

Existing unit tests - this only adds an await clause, which is safe to do to non-promises, so it shouldn't break anything.

Pull Request resolved: facebook#754

Reviewed By: motiz88

Differential Revision: D33486073

Pulled By: rh389

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

Terser v5 is async, so if we want to upgrade to support the latest syntax we need this.

**Test plan**

Existing unit tests - this only adds an await clause, which is safe to do to non-promises, so it shouldn't break anything.

Pull Request resolved: facebook#754

Reviewed By: motiz88

Differential Revision: D33486073

Pulled By: rh389

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

Terser v5 is async, so if we want to upgrade to support the latest syntax we need this.

**Test plan**

Existing unit tests - this only adds an await clause, which is safe to do to non-promises, so it shouldn't break anything.

Pull Request resolved: facebook#754

Reviewed By: motiz88

Differential Revision: D33486073

Pulled By: rh389

fbshipit-source-id: 1f6be79741144960803d018d48a51a367665708f
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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants