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 legacy chunks in IIFE (IE11) #2972

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

cztomsik
Copy link

@cztomsik cztomsik commented Apr 13, 2021

Description

template literals in our legacy chunks got randomly overwritten (gql mixed with emotion) in legacy build
the problem was that at the top of the file there were var lit1, lit2, lit3 declarations and those were evaluated in global scope. this resolves the issue

Additional context

I was surprised that systemjs does not wrap files in IIFE itself


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.

@Shinigami92 Shinigami92 added the p3-minor-bug An edge case that only affects very specific usage (priority) label Apr 13, 2021
@antfu antfu changed the title wrap legacy chunks in IIFE (IE11) fix(plugin-legacy): wrap legacy chunks in IIFE (IE11) Apr 13, 2021
antfu
antfu previously approved these changes Apr 13, 2021
template literals in our legacy chunks got randomly overwritten (gql mixed with emotion) in legacy build
the problem was that at the top of the file there were `var lit1, lit2, lit3` declarations and those were evaluated in global scope
this resolves the issue
@cztomsik
Copy link
Author

cztomsik commented Apr 14, 2021

renamed commit to fit standards, no other change
@antfu can you please re-approve when you have a moment?

patak-dev
patak-dev previously approved these changes Apr 14, 2021
@patak-dev patak-dev dismissed their stale review April 14, 2021 19:42

checking sourcemaps

@@ -260,7 +260,7 @@ function viteLegacyPlugin(options = {}) {
]
})

return { code, map }
return { code: `;(function(){${code}})();`, map }
Copy link
Member

Choose a reason for hiding this comment

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

@cztomsik shouldn't the sourcemap (map) be also updated? Using magic-string for example

Copy link
Author

Choose a reason for hiding this comment

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

good point but I'm busy now so maybe later or somebody else can fix it properly, this is just what worked for us

@antfu antfu self-assigned this Apr 17, 2021
@antfu antfu self-requested a review April 17, 2021 23:01
@patak-dev
Copy link
Member

Thanks for the PR, closing this one as #3783 was merged now.

@patak-dev patak-dev closed this Jun 13, 2021
@cztomsik
Copy link
Author

No worries, thanks @umidbekk for fixing it :)

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

Successfully merging this pull request may close these issues.

None yet

4 participants