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

[chore] upgrades html-minifier-terser@5.0.0 -> 6.0.2 #1688

Merged

Conversation

gabrielcsapo
Copy link
Contributor

Summary

fixes #1687.

Updates the snapshots to reflect the terser upgrade.

@gabrielcsapo gabrielcsapo marked this pull request as draft September 29, 2021 20:21
@gabrielcsapo gabrielcsapo marked this pull request as ready for review September 29, 2021 20:39
@gabrielcsapo
Copy link
Contributor Author

@jantimon I will have a follow up PR to make these fixtures snapshots as this would have been easier to run jest -u instead of having to update the fixtures manually if that okay with you?

@jantimon
Copy link
Owner

You can rebuild all examples with npm run rebuild-examples

Snapshots are way harder to read

@gabrielcsapo
Copy link
Contributor Author

I will run that to make sure I got everything! @jantimon

Is there anything else I should do before we can merge?

Copy link
Owner

@jantimon jantimon left a comment

Choose a reason for hiding this comment

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

We don't need async await to return the promise.
Can you please revert those two changes?

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@jantimon
Copy link
Owner

jantimon commented Oct 9, 2021

@gabrielcsapo did you already have time to look into my feedback? :)

gabrielcsapo and others added 3 commits October 9, 2021 10:23
Co-authored-by: Jan Nicklas <j.nicklas@me.com>
Co-authored-by: Jan Nicklas <j.nicklas@me.com>
@gabrielcsapo
Copy link
Contributor Author

@jantimon just updated the PR based on the feedback.

@gabrielcsapo
Copy link
Contributor Author

@jantimon kindly bumping this.

@jantimon jantimon merged commit 16a841a into jantimon:main Oct 15, 2021
@jantimon
Copy link
Owner

thanks @gabrielcsapo

@jantimon
Copy link
Owner

released as 5.4.0

@jantimon
Copy link
Owner

@gabrielcsapo your changes break the build pipelines - could you please take a look what’s wrong?

@gabrielcsapo
Copy link
Contributor Author

gabrielcsapo commented Oct 16, 2021 via email

@gabrielcsapo gabrielcsapo deleted the gabrielcsapo/upgrade-html-minifier-terser branch October 16, 2021 16:41
@gabrielcsapo
Copy link
Contributor Author

@jantimon there seems to be a race condition with checking the contents of the files on disk and starting a new test. It might make sense to have each test output to a specific directory on disk to avoid collisions. I am not seeing this same issue locally, so it will be hard to debug to be for sure.

Another option is using snapshots as they are in memory and won't have collision issues.

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.

Upgrade to Terser 5 + html-minifier-terser 6
2 participants