Skip to content

Conversation

@sapphi-red
Copy link
Member

Description

This is a regression caused by #14094

fixes #14537

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.
  • Read the Pull Request Guidelines and follow the PR Title 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.

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release labels Oct 5, 2023
Comment on lines -28 to +31
// IIFE content looks like `var MyLib = function() {`. Spaces are removed when minified
// IIFE content looks like `var MyLib = function() {`.
// Spaces are removed and parameters are mangled when minified
const IIFE_BEGIN_RE =
/(const|var)\s+\S+\s*=\s*function\(\)\s*\{.*"use strict";/s
/(const|var)\s+\S+\s*=\s*function\([^()]*\)\s*\{\s*"use strict";/
Copy link
Member Author

Choose a reason for hiding this comment

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

.* needs to be replaced with \s* otherwise this test will fail.

test('restrisct-helpers-injection', async () => {
const code = readFile(
'dist/helpers-injection/my-lib-custom-filename.iife.js',
)
expect(code).toMatch(
`'"use strict"; return (' + expressionSyntax + ").constructor;"`,
)
})

This fix relies on the fact that evanw/esbuild#3322 is fixed. But because that fix landed in 0.19.3, we cannot backport this fix to v4 directly.

We can use /(const|var)\s+\S+\s*=\s*function\([^()]*\)\s*\{[\s\w,;]*"use strict";/ instead of this regex to allow var _a, _b part that might be injected by esbuild. But I guess it's safer to simply revert #14094 for v4.

@bluwy bluwy merged commit 5004d00 into vitejs:main Oct 9, 2023
@sapphi-red sapphi-red deleted the fix/iife-esbuild-helper-injection branch October 9, 2023 08:37
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) regression The issue only appears after a new release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vite 4.4.10 - iife modules that import certain libraries break at runtime

2 participants