Skip to content

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

Merged
patak-cat merged 1 commit intovitejs:mainfrom
avocadowastaken:legacy-iife
Jun 13, 2021
Merged

fix(plugin-legacy): wrap chunks in IIFE#3783
patak-cat merged 1 commit intovitejs:mainfrom
avocadowastaken:legacy-iife

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-cat patak-cat 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-cat
Copy link
Member

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

@patak-cat
Copy link
Member

@umidbekk @vitejs/plugin-legacy@1.4.2 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