fix(vite): duplicate server style imports #33308
Conversation
|
|
BobbieGoede
changed the title
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
WalkthroughThe file packages/vite/src/plugins/ssr-styles.ts was changed to collect CSS imports and export names into sets (cssImports, exportNames, importStatements) to deduplicate entries. Import statements are generated via genImport for each unique CSS file and the default export array is emitted using genArrayFromRaw from the collected export names. The previous per-file inline import/array construction was replaced by this deduplicated, set-based emission; control flow and error handling are otherwise unchanged. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vite/src/plugins/ssr-styles.ts (1)
82-85: Avoid './..' specifiers and shadowing; keep imports clean and stable
- Prepending './' unconditionally can yield paths like './../x.mjs'. Guard it so we only prepend when needed.
- Minor: avoid shadowing the outer
filevariable in themapcallback for clarity.Apply this diff:
- const uniqueFiles = [...new Set(files.map(css => this.getFileName(css)))] - const styleImports = uniqueFiles.map((file, i) => ({ name: `style_${i}`, path: `./${relative(baseDir, file)}` })) + const uniqueFiles = [...new Set(files.map(css => this.getFileName(css)))] + const styleImports = uniqueFiles.map((chunkPath, i) => { + const rel = relative(baseDir, chunkPath) + const path = rel.startsWith('.') ? rel : `./${rel}` + return { name: `style_${i}`, path } + })Optional follow-up (broader): if duplicates are causing extra emitted chunks (not just duplicate imports), consider caching emitted chunk refs across transforms:
// at plugin top-level const emittedById = new Map<string, string>() // key: resolvedId+'?inline&used', value: ref // before emitting a chunk in transform() const chunkId = (resolved.id ?? file) + '?inline&used' const ref = emittedById.get(chunkId) ?? this.emitFile({ type: 'chunk', name: `${filename(id)}-styles-${++styleCtr}.mjs`, id: chunkId }) emittedById.set(chunkId, ref)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vite/src/plugins/ssr-styles.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/vite/src/plugins/ssr-styles.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: release-pkg-pr-new
- GitHub Check: test-size
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: code
🔇 Additional comments (2)
packages/vite/src/plugins/ssr-styles.ts (2)
4-4: LGTM on knitwork helpers importThe added imports are appropriate for the new codegen approach and used correctly below.
89-90: LGTM: deterministic imports and export generationUsing genImport and genArrayFromRaw makes the generated module concise and consistent, and pairs well with the deduped list.
CodSpeed Performance ReportMerging #33308 will not alter performanceComparing Summary
|
2 participants
🔗 Linked issue
📚 Description
Right now the server build will have style chunks that import and export the same css file as seen in the screenshot below. This also results in extra files being generated.
Screenshot of current output
Screenshot of output with this PR
Not sure if this is an ideal solution but it seems to work.