Skip to content

refactor(bundler): unify per-bundler entries into bundler factory#754

Merged
harlan-zw merged 4 commits into
mainfrom
worktree-refactor+unified-bundler-entry
Apr 24, 2026
Merged

refactor(bundler): unify per-bundler entries into bundler factory#754
harlan-zw merged 4 commits into
mainfrom
worktree-refactor+unified-bundler-entry

Conversation

@harlan-zw

@harlan-zw harlan-zw commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

🔗 Linked issue

N/A

❓ Type of change

  • 📖 Documentation
  • 🐞 Bug fix
  • 👌 Enhancement
  • ✨ New feature
  • 🧹 Chore
  • ⚠️ Breaking change

📚 Description

Replaces the per-bundler subpath exports (@unhead/{vue,react,svelte,solid-js}/webpack and @unhead/bundler/webpack) with a single bundler entry that returns a factory object exposing .vite(), .webpack(), .rspack(), and .rollup() dispatch methods. One call site now covers every supported builder, and the factory can be forwarded directly to Nuxt Kit's addBuildPlugin. Framework /vite entries stay as thin wrappers over the new factory for backwards compatibility.

⚠️ Breaking Changes

  • Removed @unhead/bundler/webpack subpath export.
  • Removed @unhead/{vue,react,svelte,solid-js}/webpack subpath exports.
  • @unhead/addons/webpack now throws at import time instead of warning.

📝 Migration

// before
import { Unhead } from '@unhead/vue/webpack'
export default { plugins: [Unhead({ streaming: true })] }

// after
import { Unhead } from '@unhead/vue/bundler'
export default { plugins: [...Unhead({ streaming: true }).webpack()] }

// nuxt kit
addBuildPlugin(Unhead({ streaming: true }))

Summary by CodeRabbit

  • New Features

    • Unified bundler API: a single framework bundler entry now exposes per-bundler dispatch (.vite(), .webpack(), .rspack(), .rollup()) for React, Vue, Svelte, and Solid.js.
  • Removals

    • Webpack-specific public entrypoints removed across packages; legacy webpack subpaths are no longer published—switch to the new framework bundler subpaths.
  • Documentation

    • Deprecation notes and usage guidance updated to point consumers to the bundler-based API.

…tory

Replace per-bundler subpath exports (`@unhead/{vue,react,svelte,solid-js}/webpack`)
with a single `bundler` entry that returns a factory object exposing
`.vite()`, `.webpack()`, `.rspack()`, and `.rollup()` dispatch methods.
Framework `vite` entries kept as thin wrappers over the new bundler factory.

The `@unhead/bundler/webpack` and `@unhead/addons/webpack` entries are removed;
consumers should import from `@unhead/<framework>/bundler` and call the matching
dispatch method (e.g. `Unhead({ streaming: true }).webpack()`), or pass the
factory directly to Nuxt Kit's `addBuildPlugin`.
@coderabbitai

coderabbitai Bot commented Apr 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 031a5025-d3e9-4f22-92b3-c158aae9978e

📥 Commits

Reviewing files that changed from the base of the PR and between 50148e1 and 2268ff8.

📒 Files selected for processing (4)
  • packages-aliased/addons/build.config.ts
  • packages-aliased/addons/package.json
  • packages-aliased/addons/src/webpack.ts
  • packages/bundler/README.md
💤 Files with no reviewable changes (4)
  • packages-aliased/addons/src/webpack.ts
  • packages-aliased/addons/build.config.ts
  • packages/bundler/README.md
  • packages-aliased/addons/package.json

📝 Walkthrough

Walkthrough

Consolidates per-framework bundler entrypoints into a unified createFrameworkPlugin factory exposing a ./bundler dispatch (vite/webpack/rspack/rollup). Removes framework-specific webpack unplugin files and ./webpack package exports; Vite wrappers delegate to the new bundler entry; package exports, build outputs, tests, and docs updated accordingly.

Changes

Cohort / File(s) Summary
Core bundler framework
packages/bundler/src/unplugin/framework.ts, packages/bundler/build.config.ts, packages/bundler/package.json
Introduce createFrameworkPlugin returning a bundler-dispatch object; remove ./webpack package export and adjust build config.
Removed webpack unplugin / addon shim
packages/bundler/src/unplugin/webpack.ts (deleted), packages-aliased/addons/src/webpack.ts
Delete webpack-specific unplugin entry; remove re-exports/deprecation warning (addon shim emptied/removed), eliminating the ./webpack export surface.
Framework bundler entries (added)
packages/*/src/bundler.ts (`packages/vue
react
Vite wrappers (refactored)
packages/*/src/vite.ts, packages/*/src/stream/vite.ts
Refactor Vite modules to call UnheadBundler(options).vite(); change Unhead export to a function returning Plugin[]; update deprecation JSDoc to recommend bundler usage.
Removed framework webpack modules
packages/*/src/webpack.ts (vue/react/svelte/solid-js deleted)
Delete framework-specific webpack integration files and their exported types/entries.
Tests, fixtures & configs
packages/*/test/unified-plugin.test.ts, packages/*/vitest.config.ts, test/exports/*.yaml, packages-aliased/*/package.json
Update tests to import unified Unhead and use .vite()/.webpack()/.rspack()/.rollup(); simplify vitest aliases; update export fixtures from ./webpack.Unhead./bundler.Unhead; remove ./webpack package exports.
Docs / JSDoc
packages/unhead/src/stream/unplugin.ts, packages/bundler/README.md
Update documentation and deprecation notes to recommend @unhead/{framework}/bundler usage and remove webpack examples.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant BundlerEntry as "framework/bundler"
    participant Factory as createFrameworkPlugin
    participant CoreResolver as resolveCoreDefs
    participant Dispatcher as UnheadBundlerFactory
    participant Vite as .vite()
    participant Webpack as .webpack()

    Dev->>BundlerEntry: import { Unhead } and call Unhead(options)
    BundlerEntry->>Factory: initialize with framework & streamingPlugin
    Factory->>CoreResolver: resolve core transforms (treeshake, seo, minify...)
    CoreResolver-->>Factory: core plugin definitions
    Factory->>Dispatcher: return dispatch object (vite/webpack/rspack/rollup)
    Dev->>Dispatcher: call .vite()
    Dispatcher->>Vite: assemble Vite plugins (validate?, devtools?, CreateHeadTransform, SSRStaticReplace, streaming...)
    Vite-->>Dev: Plugin[] for Vite
    Dev->>Dispatcher: call .webpack()
    Dispatcher->>Webpack: assemble Webpack plugins (SSRStaticReplace, streaming?)
    Webpack-->>Dev: Plugin[] for Webpack
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hopped through code to join each strand,
One factory now sends plugins hand in hand,
Vite calls .vite(), webpack finds its way,
Streaming tags along to brighten the day,
Hop hooray — one bundle, many lands!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main objective of the PR: consolidating per-bundler subpath exports into a unified bundler factory pattern.
Description check ✅ Passed The description covers all required template sections: linked issue status, type of change (breaking change), detailed explanation of changes, breaking changes list, and migration examples.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-refactor+unified-bundler-entry

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

Bundle Size Analysis

Bundle Size Gzipped
Client (Minimal) 10.7 kB 4.4 kB
Server (Minimal) 10.6 kB 4.3 kB
Vue Client (Minimal) 11.8 kB 4.9 kB
Vue Server (Minimal) 11.6 kB 4.7 kB

@harlan-zw harlan-zw changed the title refactor(bundler)!: unify per-bundler entries into bundler factory refactor(bundler): unify per-bundler entries into bundler factory Apr 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (3)
packages/unhead/src/stream/unplugin.ts (1)

282-289: Minor: the example is Vue-specific but the factory is framework-agnostic.

Since createStreamingPlugin is shared across Vue/React/Solid/Svelte, consider a framework-agnostic phrasing like "the unified framework bundler entry (e.g. @unhead/{vue,react,svelte,solid-js}/bundler)" so React/Solid/Svelte consumers aren't inadvertently steered toward the Vue subpath. Purely a doc tweak.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/unhead/src/stream/unplugin.ts` around lines 282 - 289, The comment
above the createStreamingPlugin factory is Vue-specific; update the docstring to
a framework-agnostic example so it doesn't single out Vue. Edit the block that
references createStreamingPlugin/createUnplugin (and related symbols
StreamingPluginOptions and buildStreamingPluginOptions) to replace
"`@unhead/vue/bundler`" with a neutral example such as
"`@unhead/{vue,react,svelte,solid-js}/bundler`" or a generic "the unified
framework bundler entry" phrasing so all framework consumers are equally
represented.
packages/solid-js/src/bundler.ts (1)

8-17: Optional: mention the addBuildPlugin path like the Vue entry.

The Vue bundler.ts docstring documents both the direct Vite call site and the Nuxt Kit addBuildPlugin(Unhead(...)) forwarding path, which is one of the headline wins called out in the PR description. Adding the same snippet here keeps the docs symmetrical across framework entries.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/solid-js/src/bundler.ts` around lines 8 - 17, Update the docstring
in the unified bundler plugin factory for `@unhead/solid-js` to include the Nuxt
Kit forwarding path similar to the Vue entry: add a small example showing
addBuildPlugin(Unhead({ streaming: true })) alongside the existing Vite usage
(reference the Unhead factory exported in this file and the addBuildPlugin call)
so the docs mention both direct Vite usage and the Nuxt Kit addBuildPlugin
forwarding path.
packages/svelte/src/bundler.ts (1)

8-15: Optional: add a usage @example for parity with Vue/React bundler entries.

The Vue and React bundler entries include a JSDoc @example demonstrating ...Unhead({ streaming: true }).vite(). Adding the same here (and in packages/solid-js/src/bundler.ts) would keep the public API surface discoverable and consistent across frameworks.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/svelte/src/bundler.ts` around lines 8 - 15, Add a JSDoc `@example` to
the Unhead bundler export to match Vue/React parity: update the comment above
the Unhead constant (created via createFrameworkPlugin and typed as
UnheadSvelteStreamingOptions, using unheadSvelteStreamingPlugin) to include an
example like calling Unhead({ streaming: true }).vite() so the public API usage
is discoverable and consistent across frameworks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages-aliased/addons/src/webpack.ts`:
- Around line 1-5: Update the thrown Error message in the
'@unhead/addons/webpack' removal notice to be framework-agnostic: replace the
hard-coded example '`@unhead/vue/bundler`' in the Error string with a generic
phrasing like "your framework bundler entry (e.g.
`@unhead/{vue,react,svelte,solid-js}/bundler`)" or "your framework's bundler
entry" so users of React/Svelte/Solid aren't misled; edit the string inside the
throw new Error(...) expression in the file to reflect this wording change.

In `@packages/bundler/src/unplugin/framework.ts`:
- Around line 53-58: The code treats options.minify === true as an empty object
so MinifyTransform never gets added; update the minifyOpts assignment so a
boolean true enables both JS and CSS minification: replace minifyOpts = typeof
options.minify === 'object' ? options.minify : {} with logic that if
options.minify === true set minifyOpts = { js: true, css: true }, else if it's
an object use it, otherwise {} — then the existing guard (if (minifyOpts.js ||
minifyOpts.css)) will correctly push { instance: MinifyTransform, options: {
...common, ...minifyOpts } } into defs.
- Around line 110-147: UnheadFrameworkOptions currently exposes validate and
devtools but those options are only honored in the .vite() branch (see
CreateHeadTransform injection of ValidatePlugin and unheadDevtools usage), so
update the type definition for UnheadFrameworkOptions to either remove validate
(if you want it unavailable outside Vite) or add explicit JSDoc noting that
validate and devtools are Vite-only and will be ignored by .webpack(),
.rspack(), and .rollup(); ensure the JSDoc mentions the related symbols
(CreateHeadTransform, ValidatePlugin, unheadDevtools) and that only .vite()
consumes these options so callers aren’t misled.

In `@packages/react/test/unified-plugin.test.ts`:
- Around line 25-29: The test currently only checks for any plugin with an apply
function but must assert the actual streaming plugin is present; update the test
in unified-plugin.test.ts (the Unhead({ streaming: true }).webpack() call) to
search the returned plugins array for a streaming-specific marker instead of a
generic apply check — for example look for a unique identifier on the plugin
such as its constructor name ("UnheadStreamingPlugin"), a public name property
(plugin.name === "UnheadStreamingPlugin"), or a dedicated flag (e.g.
plugin.__unheadStreaming) and assert that such a plugin exists; this ensures the
streaming webpack plugin introduced by
createFrameworkPlugin/Unhead(...).webpack() is actually included when streaming:
true.

In `@packages/vue/src/vite.ts`:
- Around line 11-16: Update the `@example` to reflect that Unhead now returns
Plugin[]: change the example usage of Unhead in vite integration so it spreads
the returned array into the plugins array (matching the bundler.ts and migration
docs) or remove the example; specifically update the example line that currently
shows plugins: [Unhead({ streaming: true })] to use the spread form plugins:
[...Unhead({ streaming: true })] (referencing the Unhead function and the
Plugin[] return shape).

---

Nitpick comments:
In `@packages/solid-js/src/bundler.ts`:
- Around line 8-17: Update the docstring in the unified bundler plugin factory
for `@unhead/solid-js` to include the Nuxt Kit forwarding path similar to the
Vue entry: add a small example showing addBuildPlugin(Unhead({ streaming: true
})) alongside the existing Vite usage (reference the Unhead factory exported in
this file and the addBuildPlugin call) so the docs mention both direct Vite
usage and the Nuxt Kit addBuildPlugin forwarding path.

In `@packages/svelte/src/bundler.ts`:
- Around line 8-15: Add a JSDoc `@example` to the Unhead bundler export to match
Vue/React parity: update the comment above the Unhead constant (created via
createFrameworkPlugin and typed as UnheadSvelteStreamingOptions, using
unheadSvelteStreamingPlugin) to include an example like calling Unhead({
streaming: true }).vite() so the public API usage is discoverable and consistent
across frameworks.

In `@packages/unhead/src/stream/unplugin.ts`:
- Around line 282-289: The comment above the createStreamingPlugin factory is
Vue-specific; update the docstring to a framework-agnostic example so it doesn't
single out Vue. Edit the block that references
createStreamingPlugin/createUnplugin (and related symbols StreamingPluginOptions
and buildStreamingPluginOptions) to replace "`@unhead/vue/bundler`" with a
neutral example such as "`@unhead/{vue,react,svelte,solid-js}/bundler`" or a
generic "the unified framework bundler entry" phrasing so all framework
consumers are equally represented.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e62317cf-ec4a-48e9-8c02-8560500403cf

📥 Commits

Reviewing files that changed from the base of the PR and between 3ed063c and 5dc341c.

📒 Files selected for processing (43)
  • packages-aliased/addons/src/webpack.ts
  • packages/bundler/build.config.ts
  • packages/bundler/package.json
  • packages/bundler/src/unplugin/framework.ts
  • packages/bundler/src/unplugin/webpack.ts
  • packages/react/build.config.ts
  • packages/react/package.json
  • packages/react/src/bundler.ts
  • packages/react/src/stream/vite.ts
  • packages/react/src/vite.ts
  • packages/react/src/webpack.ts
  • packages/react/test/unified-plugin.test.ts
  • packages/react/vitest.config.ts
  • packages/solid-js/build.config.ts
  • packages/solid-js/package.json
  • packages/solid-js/src/bundler.ts
  • packages/solid-js/src/stream/vite.ts
  • packages/solid-js/src/vite.ts
  • packages/solid-js/src/webpack.ts
  • packages/solid-js/test/unified-plugin.test.ts
  • packages/solid-js/vitest.config.ts
  • packages/svelte/build.config.ts
  • packages/svelte/package.json
  • packages/svelte/src/bundler.ts
  • packages/svelte/src/stream/vite.ts
  • packages/svelte/src/vite.ts
  • packages/svelte/src/webpack.ts
  • packages/svelte/test/unified-plugin.test.ts
  • packages/svelte/vitest.config.ts
  • packages/unhead/src/stream/unplugin.ts
  • packages/vue/build.config.ts
  • packages/vue/package.json
  • packages/vue/src/bundler.ts
  • packages/vue/src/stream/vite.ts
  • packages/vue/src/vite.ts
  • packages/vue/src/webpack.ts
  • packages/vue/test/unified-plugin.test.ts
  • packages/vue/vitest.config.ts
  • test/exports/bundler.yaml
  • test/exports/react.yaml
  • test/exports/solid-js.yaml
  • test/exports/svelte.yaml
  • test/exports/vue.yaml
💤 Files with no reviewable changes (11)
  • packages/bundler/build.config.ts
  • packages/svelte/vitest.config.ts
  • packages/react/src/webpack.ts
  • packages/svelte/src/webpack.ts
  • packages/solid-js/vitest.config.ts
  • packages/vue/src/webpack.ts
  • packages/bundler/package.json
  • packages/vue/vitest.config.ts
  • packages/react/vitest.config.ts
  • packages/bundler/src/unplugin/webpack.ts
  • packages/solid-js/src/webpack.ts

Comment thread packages-aliased/addons/src/webpack.ts Outdated
Comment thread packages/bundler/src/unplugin/framework.ts
Comment thread packages/bundler/src/unplugin/framework.ts
Comment on lines 25 to 29
it('webpack returns a plugin array including the streaming plugin when enabled', () => {
const plugins = UnheadWebpack({ streaming: true }) as any[]
const plugins = Unhead({ streaming: true }).webpack() as any[]
expect(Array.isArray(plugins)).toBe(true)
expect(plugins.some(p => typeof p?.apply === 'function')).toBe(true)
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Minor: webpack assertion does not specifically verify the streaming plugin is present.

The test title says "including the streaming plugin when enabled", but the assertion only checks that some plugin with an apply function exists — which is true even without streaming (e.g., SSRStaticReplace.webpack). Consider asserting on a streaming-specific marker (e.g., by name or instance type) to match the test description and guard against regressions where the streaming webpack plugin is dropped from createFrameworkPlugin's webpack() branch.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/test/unified-plugin.test.ts` around lines 25 - 29, The test
currently only checks for any plugin with an apply function but must assert the
actual streaming plugin is present; update the test in unified-plugin.test.ts
(the Unhead({ streaming: true }).webpack() call) to search the returned plugins
array for a streaming-specific marker instead of a generic apply check — for
example look for a unique identifier on the plugin such as its constructor name
("UnheadStreamingPlugin"), a public name property (plugin.name ===
"UnheadStreamingPlugin"), or a dedicated flag (e.g. plugin.__unheadStreaming)
and assert that such a plugin exists; this ensures the streaming webpack plugin
introduced by createFrameworkPlugin/Unhead(...).webpack() is actually included
when streaming: true.

Comment thread packages/vue/src/vite.ts
…options

Address CodeRabbit findings:

- `minify: true` now enables both JS + CSS minification instead of
  silently no-oping (asymmetric with `treeshake: true`/`transformSeoMeta: true`).
- Document `validate` and `devtools` as Vite-only on
  `UnheadFrameworkOptions`; webpack/rspack/rollup ignore them.
- Make `@unhead/addons/webpack` removal error framework-agnostic.
- Strengthen webpack streaming-plugin tests via baseline count delta
  (unplugin webpack wrappers expose only `.apply`, no `name`).
- Fix `@unhead/<fw>/vite` example to spread the returned `Plugin[]`.
- Reword streaming unplugin doc to cover all framework bundler entries.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bundler/src/unplugin/framework.ts`:
- Around line 145-162: UnheadBundlerFactory.rollup currently calls
dispatch('rollup', defs) and then SSRStaticReplace.rollup({}) which leaves
SSRStaticReplace's internal ssr flag at its default false; update the rollup
path so SSRStaticReplace receives the correct SSR signal by either (A) plumbing
an explicit ssr option through dispatch/defs and passing it into
SSRStaticReplace.rollup({ ssr }) or (B) implementing a rollup-specific hook
inside SSRStaticReplace that inspects rollup build options (e.g.,
options.output.format or other env hints) to set ssr, and ensure
streamingPlugin.rollup(streamOpts) remains unchanged; if rollup SSR cannot be
supported, add a clear JSDoc note on UnheadBundlerFactory.rollup documenting
that SSR is not supported.
- Around line 138-162: The streaming plugin results are pushed directly
(streamingPlugin.vite/webpack/rspack/rollup) which can produce either a single
plugin or an array; mirror the dispatch() handling by normalizing before
pushing: call the streamingPlugin.* method, then if the result is an array
spread it into plugins, otherwise if truthy push the single item. Apply this
normalization to the vite, webpack, rspack and rollup branches so plugins stays
flat (instead of changing FrameworkPluginConfig.streamingPlugin types).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d1fac7d4-8baa-4c4d-b47c-dbcf5ea50d48

📥 Commits

Reviewing files that changed from the base of the PR and between 5dc341c and c7ccae2.

📒 Files selected for processing (8)
  • packages-aliased/addons/src/webpack.ts
  • packages/bundler/src/unplugin/framework.ts
  • packages/react/test/unified-plugin.test.ts
  • packages/solid-js/test/unified-plugin.test.ts
  • packages/svelte/test/unified-plugin.test.ts
  • packages/unhead/src/stream/unplugin.ts
  • packages/vue/src/vite.ts
  • packages/vue/test/unified-plugin.test.ts
✅ Files skipped from review due to trivial changes (2)
  • packages-aliased/addons/src/webpack.ts
  • packages/unhead/src/stream/unplugin.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • packages/vue/src/vite.ts
  • packages/vue/test/unified-plugin.test.ts
  • packages/solid-js/test/unified-plugin.test.ts
  • packages/svelte/test/unified-plugin.test.ts
  • packages/react/test/unified-plugin.test.ts

Comment thread packages/bundler/src/unplugin/framework.ts Outdated
Comment on lines +145 to +162
plugins.push(SSRStaticReplace.webpack({}))
if (wantStreaming)
plugins.push(streamingPlugin.webpack(streamOpts))
return plugins
},
rspack: () => {
const plugins = dispatch('rspack', defs)
plugins.push(SSRStaticReplace.rspack({}))
if (wantStreaming)
plugins.push(streamingPlugin.rspack(streamOpts))
return plugins
},
rollup: () => {
const plugins = dispatch('rollup', defs)
plugins.push(SSRStaticReplace.rollup({}))
if (wantStreaming)
plugins.push(streamingPlugin.rollup(streamOpts) as any)
return plugins

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm no rollup-specific SSR detection exists for SSRStaticReplace and check
# whether any caller relies on rollup SSR in this repo.
fd -t f 'SSRStaticReplace.ts' packages/bundler/src
rg -nP -C3 '\b(rollup|isSsrBuild|ctx\.name)\b' packages/bundler/src/unplugin/SSRStaticReplace.ts
rg -nP -C2 '\.rollup\(' packages --type ts -g '!**/node_modules/**'

Repository: unjs/unhead

Length of output: 4231


🏁 Script executed:

# Read the full SSRStaticReplace.ts to understand how the ssr flag is used
cat -n packages/bundler/src/unplugin/SSRStaticReplace.ts | head -100

Repository: unjs/unhead

Length of output: 2181


🏁 Script executed:

# Check if there's any rollup-specific handling in the dispatch function or framework context
rg -nP -A5 "dispatch\('rollup'" packages/bundler/src/unplugin/

Repository: unjs/unhead

Length of output: 561


🏁 Script executed:

# Search for any rollup SSR builds or configuration in tests/docs
rg -nP "rollup.*ssr|ssr.*rollup" packages --type ts --type js

Repository: unjs/unhead

Length of output: 37


🏁 Script executed:

# Check the dispatch function definition and what it returns for rollup
rg -nP -B5 -A10 "function dispatch|const dispatch" packages/bundler/src/unplugin/framework.ts

Repository: unjs/unhead

Length of output: 463


🏁 Script executed:

# Look for any unplugin documentation or examples on how rollup context is typically accessed
rg -nP "rollup.*options|options\.output" packages/bundler/src --type ts

Repository: unjs/unhead

Length of output: 37


🏁 Script executed:

# Check if there are any rollup-specific build tests that verify SSR behavior
fd -t f '\.test\.|\.spec\.' packages --type ts | xargs grep -l "rollup.*ssr\|ssr.*build" 2>/dev/null | head -5

Repository: unjs/unhead

Length of output: 225


🏁 Script executed:

# Check if there are rollup SSR tests or configuration
find packages -name '*.test.*' -o -name '*.spec.*' | xargs grep -l "rollup" | head -10

Repository: unjs/unhead

Length of output: 258


🏁 Script executed:

# Look for any rollup configuration or entry points in package.json files
fd 'package.json' packages --max-depth 2 | xargs grep -l "rollup" | head -5

Repository: unjs/unhead

Length of output: 114


🏁 Script executed:

# Search for any mention of rollup SSR support in documentation or comments
rg -nP "rollup.*SSR|rollup.*ssr|ssr.*rollup" packages/bundler/src --type ts -i

Repository: unjs/unhead

Length of output: 152


Rollup path unconditionally replaces head.ssr with false.

SSRStaticReplace detects SSR builds via the webpack(ctx) hook (checking ctx.name === 'server') and the vite.apply hook (checking env.isSsrBuild). The rollup export has no hook to detect SSR context, leaving the internal ssr flag at its default false value. Consequently, all head.ssr references in unhead modules are statically rewritten to false during rollup builds, regardless of whether it's an SSR build.

If rollup SSR is an intended use case, SSRStaticReplace needs a way to receive the SSR signal—either by plumbing ssr as an explicit option through dispatch() or by implementing a rollup-specific hook that inspects options.output.format or environment hints. If rollup SSR is not supported, document it in the UnheadBundlerFactory.rollup JSDoc so users know what to expect.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bundler/src/unplugin/framework.ts` around lines 145 - 162,
UnheadBundlerFactory.rollup currently calls dispatch('rollup', defs) and then
SSRStaticReplace.rollup({}) which leaves SSRStaticReplace's internal ssr flag at
its default false; update the rollup path so SSRStaticReplace receives the
correct SSR signal by either (A) plumbing an explicit ssr option through
dispatch/defs and passing it into SSRStaticReplace.rollup({ ssr }) or (B)
implementing a rollup-specific hook inside SSRStaticReplace that inspects rollup
build options (e.g., options.output.format or other env hints) to set ssr, and
ensure streamingPlugin.rollup(streamOpts) remains unchanged; if rollup SSR
cannot be supported, add a clear JSDoc note on UnheadBundlerFactory.rollup
documenting that SSR is not supported.

… limitation

Address remaining CodeRabbit findings:

- `pushPlugin` helper flattens streaming plugin results so an
  `UnpluginInstance<S, true>` (Nested) wouldn't leak nested arrays into
  the dispatched output, matching the existing `dispatch` helper's
  array handling.
- Document on `UnheadBundlerFactory` that `.rollup()` cannot detect SSR
  context, so `head.ssr` references are always rewritten to `false`.
  Use `.vite()` or `.webpack()` for SSR builds.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/bundler/src/unplugin/framework.ts (1)

58-95: Optional: tighten any usage in CoreDef/dispatch.

CoreDef.options: any and the instance[bundler] as (opts: any) => any cast discard the option/plugin shape that each transform's UnpluginInstance already encodes. Making CoreDef generic and inferring the per-bundler return type from UnpluginInstance<O, false> would preserve type safety across the dispatch pipeline without changing runtime behavior.

♻️ Sketch
-interface CoreDef { instance: UnpluginInstance<any, false>, options: any }
+interface CoreDef<O = unknown> {
+  instance: UnpluginInstance<O, false>
+  options: O
+}
@@
-function dispatch(bundler: 'vite' | 'webpack' | 'rspack' | 'rollup', defs: CoreDef[]): any[] {
-  const out: any[] = []
-  for (const { instance, options } of defs) {
-    const plugin = (instance[bundler] as (opts: any) => any)(options)
+type Bundler = 'vite' | 'webpack' | 'rspack' | 'rollup'
+function dispatch(bundler: Bundler, defs: CoreDef[]): unknown[] {
+  const out: unknown[] = []
+  for (const { instance, options } of defs) {
+    const plugin = instance[bundler](options)
     if (Array.isArray(plugin))
       out.push(...plugin)
     else out.push(plugin)
   }
   return out
 }

Leaving this optional since the current shape works and the factory's public signature (UnheadBundlerFactory) is the contract consumers depend on.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/bundler/src/unplugin/framework.ts` around lines 58 - 95, Make
CoreDef and related functions generic so you preserve each plugin's option and
return types instead of using any: change CoreDef to parameterize the
options/instance types (e.g., CoreDef<O, R> with instance: UnpluginInstance<O,
false> and options: O), update resolveCoreDefs to return CoreDef<...>[] where
the concrete option types come from the specific instances you construct
(TreeshakeServerComposables, UseSeoMetaTransform, MinifyTransform), and update
dispatch to infer the bundler-specific plugin factory signature from
UnpluginInstance so you can call instance[bundler](options) without casting to
any and return the correct aggregated array type; this removes the (opts: any)
=> any cast while keeping runtime behavior unchanged and preserves per-plugin
type safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/bundler/src/unplugin/framework.ts`:
- Around line 58-95: Make CoreDef and related functions generic so you preserve
each plugin's option and return types instead of using any: change CoreDef to
parameterize the options/instance types (e.g., CoreDef<O, R> with instance:
UnpluginInstance<O, false> and options: O), update resolveCoreDefs to return
CoreDef<...>[] where the concrete option types come from the specific instances
you construct (TreeshakeServerComposables, UseSeoMetaTransform,
MinifyTransform), and update dispatch to infer the bundler-specific plugin
factory signature from UnpluginInstance so you can call
instance[bundler](options) without casting to any and return the correct
aggregated array type; this removes the (opts: any) => any cast while keeping
runtime behavior unchanged and preserves per-plugin type safety.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 98b72596-9e1a-4adc-a89e-e3ce7ccbbc70

📥 Commits

Reviewing files that changed from the base of the PR and between c7ccae2 and 50148e1.

📒 Files selected for processing (1)
  • packages/bundler/src/unplugin/framework.ts

The /webpack subpaths were never published to npm, so we can drop the
addons/webpack throw stub and its README example without a deprecation
window.
@harlan-zw harlan-zw merged commit 5257d27 into main Apr 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant