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

fix(plugin-legacy): wrap chunks in IIFE #3783

Merged
merged 1 commit into from
Jun 13, 2021
Merged

fix(plugin-legacy): wrap chunks in IIFE #3783

merged 1 commit into from
Jun 13, 2021

Conversation

avocadowastaken
Copy link
Contributor

@avocadowastaken avocadowastaken commented Jun 13, 2021

Description

Solving the same problem as #2972, but contains tests and source map support (sorry for the duplicate, but original PR is not changed since the April and this bug block migration of the large codebase from the CRA to Vite).

If someone comes from google, it solves global scope pollution issues like:

  • Can't create duplicate variable: '_excludes', redeclaration of const _excludes or Identifier '_excludes' has already been declared errors caused by @babel/plugin-desctructuring (example)

  • Issues with styled-components or emotion caused by @babel/plugin-transform-template-literals (example)

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.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • 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).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@antfu
Copy link
Member

antfu commented Jun 13, 2021

I think we would need to keep the map from babel, and that's why the original PR get blocked since MagicString does not take source map input, while I am not familiar with babel transforming to support sourcemap

@avocadowastaken
Copy link
Contributor Author

@antfu thanks for the quick review! I've changed implementation to use babel plugin for transformations, so source map behavior should not change

Copy link
Member

@antfu antfu left a comment

Choose a reason for hiding this comment

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

Thanks!

@patak-dev patak-dev merged commit 9abdb81 into vitejs:main Jun 13, 2021
@avocadowastaken avocadowastaken deleted the legacy-iife branch June 13, 2021 12:02
@avocadowastaken
Copy link
Contributor Author

@antfu @patak-js sorry if I'm being annoying, but is there any release plan for that fix?

@patak-dev
Copy link
Member

We'll do a release on Monday or Tuesday including this fix

@patak-dev
Copy link
Member

@umidbekk @vitejs/[email protected] has been released including this PR

javastation pushed a commit to javastation/vite that referenced this pull request Jun 22, 2021
aleclarson pushed a commit to aleclarson/vite that referenced this pull request Jun 25, 2021
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.

3 participants