Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(plugin-legacy): add invoke to modern detector to avoid terser treeshaking #14968

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

donaldyu
Copy link
Contributor

Description

fixdetectModernBrowserDetector in @vite/plugin-legacy.Because terser will remove empty functions that are not being called which will cause detector different between html and entry file.For example:

image image

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines, especially the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Update the corresponding documentation if needed.
  • Ideally, include relevant tests that fail without this PR but pass with it.

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

LGTM. Can you also run

<!--
Run `node --input-type=module -e "import {cspHashes} from '@vitejs/plugin-legacy'; console.log(cspHashes.map(h => 'sha256-'+h))"` to retrieve the value.
-->
to update the CSP hashes?

@bluwy bluwy changed the title fix: add invoke to avoid terser clear modern detector fix(plugin-legacy): add invoke to modern detector to avoid terser treeshaking Nov 13, 2023
@bluwy bluwy added plugin: legacy p3-minor-bug An edge case that only affects very specific usage (priority) labels Nov 13, 2023
@donaldyu
Copy link
Contributor Author

done

@patak-dev patak-dev merged commit 4033a32 into vitejs:main Nov 13, 2023
@@ -156,7 +156,7 @@ The legacy plugin requires inline scripts for [Safari 10.1 `nomodule` fix](https

- `sha256-MS6/3FCg4WjP9gwgaBGwLpRCY6fZBgwmhVCdrPrNf3E=`
- `sha256-tQjf8gvb2ROOMapIxFvFAYBeUJ0v1HCbOcSmDNXGtDo=`
- `sha256-4y/gEB2/KIwZFTfNqwXJq4olzvmQ0S214m9jwKgNXoc=`
- `sha256-8uUkKieevHiD3yYtzjkRvyDZWt+uZkBLuGEQWNiV3+c=`
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing hash is a breaking(!) change. Should be mentioned in changelog.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
p3-minor-bug An edge case that only affects very specific usage (priority) plugin: legacy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants