perf(nuxt): use prerelease bundle renderer with precompute support #33361
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33361 will not alter performanceComparing Summary
Footnotes |
WalkthroughAdds generation and consumption of precomputed client dependency data and a Nitro virtual module for entry IDs. Vite and Webpack now emit client.manifest.mjs and client.precomputed.mjs (using precomputeDependencies and serialize). Vite additionally creates and wires a Nitro virtual module '#internal/nuxt/entry-ids.mjs' containing entry IDs; Nitro core also registers a default virtual entry-ids module. Renderer code now imports a static entryIds module and conditionally loads precomputed data (manifest mainly in dev). Webpack client plugin and build outputs are routed into dist/server. Tests updated for a bundle size change. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (2)
packages/vite/src/manifest.ts (1)
69-72: Parity: also emit client.manifest.json in Vite for DX/toolingWebpack writes client.manifest.json alongside .mjs files; Vite writes only .mjs. Emitting JSON here keeps outputs consistent and helps debugging and tooling.
Apply:
const precomputed = precomputeDependencies(manifest) - await writeFile(resolve(serverDist, 'client.manifest.mjs'), 'export default ' + serialize(manifest), 'utf8') + await writeFile(resolve(serverDist, 'client.manifest.json'), JSON.stringify(manifest, null, 2), 'utf8') + await writeFile(resolve(serverDist, 'client.manifest.mjs'), 'export default ' + serialize(manifest), 'utf8') await writeFile(resolve(serverDist, 'client.precomputed.mjs'), 'export default ' + serialize(precomputed), 'utf8')packages/webpack/src/presets/vue.ts (1)
18-24: Optional: set entry IDs via build:manifest hook for Webpack (central place)You can mirror the Vite approach here to keep logic out of the plugin class.
Outside the shown hunk, add:
// at top import { useNitro } from '@nuxt/kit' // inside vue(ctx) if (!ctx.nuxt.options.dev) { const nitro = useNitro() ctx.nuxt.hook('build:manifest', (manifest: any) => { const ids = Object.values(manifest) .filter((c: any) => c?.isEntry && c?.src) .map((c: any) => c.src) const mod = () => `export default ${JSON.stringify(ids)}` nitro.options.virtual['#internal/nuxt/entry-ids.mjs'] = mod nitro.options._config.virtual ||= {} nitro.options._config.virtual['#internal/nuxt/entry-ids.mjs'] = mod }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/nuxt/package.jsonis excluded by!**/package.jsonpackages/rspack/package.jsonis excluded by!**/package.jsonpackages/vite/package.jsonis excluded by!**/package.jsonpackages/webpack/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (9)
packages/nuxt/src/core/nitro.ts(1 hunks)packages/nuxt/src/core/runtime/nitro/handlers/renderer.ts(3 hunks)packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts(4 hunks)packages/vite/src/manifest.ts(2 hunks)packages/vite/src/vite-node.ts(1 hunks)packages/vite/src/vite.ts(1 hunks)packages/webpack/src/plugins/vue/client.ts(2 hunks)packages/webpack/src/presets/vue.ts(1 hunks)test/bundle.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/core/nitro.tspackages/nuxt/src/core/runtime/nitro/handlers/renderer.tspackages/vite/src/vite.tstest/bundle.test.tspackages/vite/src/vite-node.tspackages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.tspackages/vite/src/manifest.tspackages/webpack/src/presets/vue.tspackages/webpack/src/plugins/vue/client.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
test/bundle.test.ts
🧠 Learnings (1)
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/webpack/src/presets/vue.ts
🧬 Code graph analysis (2)
packages/vite/src/vite.ts (1)
packages/kit/src/index.ts (1)
useNitro(37-37)
packages/webpack/src/presets/vue.ts (1)
packages/webpack/src/plugins/vue/client.ts (1)
VueSSRClientPlugin(22-143)
⏰ 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). (7)
- GitHub Check: test-fixtures (windows-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, 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, built, rspack, async, manifest-on, json, lts/-1)
🔇 Additional comments (9)
packages/webpack/src/presets/vue.ts (1)
19-20: Plugin options change looks correctDropping filename aligns with the client plugin writing to serverDist.
packages/nuxt/src/core/runtime/nitro/handlers/renderer.ts (1)
142-145: Potential duplicate CSS when inlining entry chunk CSSWith inline styles, entryIds are forced into ssrContext.modules. In Vite, non-entry chunks have CSS lists cleared, but entry chunks keep CSS. This may render inlined CSS plus a stylesheet link for the entry chunk. Please double‑check duplication on a minimal page.
Minimal check suggestion: inspect head output for duplicate CSS when features.inlineStyles = true.
packages/vite/src/manifest.ts (1)
7-11: Update package constraints for vue-bundle-renderer and seroval
Require versions that export precomputeDependencies and serialize (by adding or bumping the minimum version in package.json) to prevent missing-API build/runtime failures.packages/nuxt/src/core/nitro.ts (1)
149-150: Approve default stub for entry IDs
Vite plugin (packages/vite/src/vite.ts:267) populates real entry IDs, overriding this fallback in production.packages/vite/src/vite.ts (1)
253-270: Centralised entry IDs + Nitro virtual wiring looks good
Entry chunks retain CSS (chunk.css &&= []only applies to non-entry); verify with a simple route that inlining doesn’t duplicate styles.packages/vite/src/vite-node.ts (1)
506-514: Dev stub acceptable due to runtime guard
The import.meta.dev check ensures the undefined stub is only used in development and enforces a real precomputed file in production. No changes required.test/bundle.test.ts (1)
61-61: Snapshot bumps look fine; confirm stability across OS/Node environments
The ∼+1 k increases in all threedefault server bundle sizetests are expected with precompute/seroval. Run the full test suite locally (Node v24.3.0 on macOS/Linux) and in CI to verify consistent snapshot outputs. If you observe drift or flaky failures, replace strict inline snapshots with a range-based assertion (e.g. allow ±1 k).packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts (2)
28-31: LGTM: precomputed loader mirrors manifest loader.Implementation is consistent and resilient to default/function exports.
39-47: Revert theprecomputedfallback — unsupported by vue-bundle-renderer v2.2.0
vue-bundle-renderer’screateRendererAPI in v2.x does not accept aprecomputedoption; the proposed diff will break renderer creation.Likely an incorrect or invalid review comment.
packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts (1)
64-86: SPA: mirror SSR fallback and catch precomputed import errors.Without this, SPA can fail when precomputed is disabled/not generated.
- const precomputed = import.meta.dev ? undefined : await getPrecomputedDependencies() + const precomputed = import.meta.dev ? undefined : await getPrecomputedDependencies().catch(() => undefined) + const manifest = (import.meta.dev || !precomputed) ? await getClientManifest().catch(() => undefined) : undefined @@ - const renderer = createRenderer(() => () => {}, { - precomputed, - manifest: import.meta.dev ? await getClientManifest() : undefined, + const renderer = createRenderer(() => () => {}, { + precomputed, + manifest, renderToString: () => spaTemplate, buildAssetsURL, })
🧹 Nitpick comments (2)
packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts (2)
22-26: Drop redundant type assertion on the promise.The return type is already declared as Promise. The trailing assertion is unnecessary.
- .then(r => typeof r === 'function' ? r() : r) as Promise<Manifest> + .then(r => typeof r === 'function' ? r() : r)
27-30: Loader normalisation duplicated; consider a tiny helper.Both loaders share the same default-or-call pattern. Extracting a small normalise helper keeps them in sync.
Example (outside this range):
const normalise = <T>(p: Promise<any>): Promise<T> => p.then((r) => r.default || r).then((r) => (typeof r === 'function' ? r() : r))Usage:
const getClientManifest = () => normalise<Manifest>(import('#build/dist/server/client.manifest.mjs')) const getPrecomputedDependencies = () => normalise<PrecomputedData>(import('#build/dist/server/client.precomputed.mjs'))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/nuxt/package.jsonis excluded by!**/package.jsonpackages/rspack/package.jsonis excluded by!**/package.jsonpackages/vite/package.jsonis excluded by!**/package.jsonpackages/webpack/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (2)
packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts(4 hunks)test/bundle.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/bundle.test.ts
🧰 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/nuxt/src/core/runtime/nitro/utils/renderer/build-files.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). (2)
- GitHub Check: build
- GitHub Check: code
🔇 Additional comments (1)
packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts (1)
4-4: Import source correction looks good.Using types from vue-bundle-renderer (not Vite) aligns with renderer expectations.
| // Load precomputed dependencies | ||
| const precomputed = import.meta.dev ? undefined : await getPrecomputedDependencies() | ||
|
|
||
| // Create renderer | ||
| const renderer = createRenderer(createSSRApp, { | ||
| precomputed, | ||
| manifest: import.meta.dev ? await getClientManifest() : undefined, | ||
| renderToString, | ||
| buildAssetsURL, | ||
| } | ||
| // Create renderer | ||
| const renderer = createRenderer(createSSRApp, options) | ||
| }) |
There was a problem hiding this comment.
SSR: add graceful fallback (precomputed → manifest) and handle missing precomputed.
Precomputed may be opt-out or absent; avoid crashing and fall back to manifest.
- // Load precomputed dependencies
- const precomputed = import.meta.dev ? undefined : await getPrecomputedDependencies()
+ // Load precomputed dependencies (graceful fallback)
+ const precomputed = import.meta.dev ? undefined : await getPrecomputedDependencies().catch(() => undefined)
+ const manifest = (import.meta.dev || !precomputed) ? await getClientManifest().catch(() => undefined) : undefined
@@
- const renderer = createRenderer(createSSRApp, {
- precomputed,
- manifest: import.meta.dev ? await getClientManifest() : undefined,
+ const renderer = createRenderer(createSSRApp, {
+ precomputed,
+ manifest,
renderToString,
buildAssetsURL,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Load precomputed dependencies | |
| const precomputed = import.meta.dev ? undefined : await getPrecomputedDependencies() | |
| // Create renderer | |
| const renderer = createRenderer(createSSRApp, { | |
| precomputed, | |
| manifest: import.meta.dev ? await getClientManifest() : undefined, | |
| renderToString, | |
| buildAssetsURL, | |
| } | |
| // Create renderer | |
| const renderer = createRenderer(createSSRApp, options) | |
| }) | |
| // Load precomputed dependencies (graceful fallback) | |
| const precomputed = import.meta.dev | |
| ? undefined | |
| : await getPrecomputedDependencies().catch(() => undefined) | |
| const manifest = (import.meta.dev || !precomputed) | |
| ? await getClientManifest().catch(() => undefined) | |
| : undefined | |
| // Create renderer | |
| const renderer = createRenderer(createSSRApp, { | |
| precomputed, | |
| manifest, | |
| renderToString, | |
| buildAssetsURL, | |
| }) |
🔗 Linked issue
📚 Description
this updates to use a precomputed dependency graph (nuxt-contrib/vue-bundle-renderer#270).
we adopt
serovalto deduplicate common strings and significantly save on the otherwise increased space the precomputed dependencies would take up in the server bundle.we might ship this as an opt-out feature.