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): group discovered polyfills by output #17347

Merged
merged 2 commits into from
May 30, 2024
Merged

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented May 29, 2024

Description

Since merging #16566, it caused CI to be flaky as in Rollup watch mode, the renderStart and renderChunk hooks are all running overlapping due to Rollup writing each output in parallel: https://github.com/rollup/rollup/blob/1a7da5ac529d543ad8fffd1cdfbf9a80040a1176/src/watch/watch.ts#L205-L216

In contrary to Vite where we write each output serially:

const res: RollupOutput[] = []
for (const output of normalizedOutputs) {
res.push(await bundle[options.write ? 'write' : 'generate'](output))
}

If we want to, we can fix watch mode to write serially manually, but I figured to fix this in plugin-legacy itself first. By grouping each output to its own map, we can handle the chunks in renderChunk in parallel.

I also updated the code to use the variables defensively to prevent issues like this in the future. I needed to hook into the Rollup watcher error event in order to get the error message:

  error: [vite:legacy-post-process] [BABEL] unknown file: Cannot read properties of undefined (reading 'legacy') (While processing: "base$0")
      at apply (file:///Users/bjorn/Work/oss/vite/packages/plugin-legacy/dist/index.mjs:454:72)
      at sync (/Users/bjorn/Work/oss/vite/node_modules/.pnpm/@[email protected]/node_modules/@babel/core/src/gensync-utils/async.ts:31:25)
      at sync (/Users/bjorn/Work/oss/vite/node_modules/.pnpm/[email protected]/node_modules/gensync/index.js:182:19)
      at factory (/Users/bjorn/Work/oss/vite/node_modules/.pnpm/[email protected]/node_modules/gensync/index.js:210:24)
      at Generator.next (<anonymous>)
      at /Users/bjorn/Work/oss/vite/node_modules/.pnpm/@[email protected]/node_modules/@babel/core/src/config/full.ts:271:23
      at Generator.next (<anonymous>)

Copy link

stackblitz bot commented May 29, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@bluwy bluwy added plugin: legacy p3-minor-bug An edge case that only affects very specific usage (priority) labels May 30, 2024
@bluwy bluwy merged commit c735cc7 into main May 30, 2024
11 checks passed
@bluwy bluwy deleted the legacy-fix-flaky branch May 30, 2024 07:06
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) plugin: legacy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants