perf(unhead): optimize SSR rendering hot paths - #940
Closed
harlan-zw wants to merge 12 commits into
Closed
Conversation
📝 WalkthroughWalkthroughUnhead now shares hook-aware SSR orchestration, supports validated and trusted serialization, improves resolver and deduplication handling, and serializes server event handlers. Schema.org adds snapshot-based SSR graph caching. Performance tooling now propagates failures and reports confidence intervals. ChangesUnhead rendering and resolution
Schema.org SSR graph cache
CI performance reporting
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant createServerRenderer
participant renderServerHead
participant resolveTags
participant ssrRenderTags
participant ssrRenderTagsTrusted
createServerRenderer->>renderServerHead: render head
renderServerHead->>renderServerHead: run lifecycle hooks
renderServerHead->>resolveTags: resolve and deduplicate tags
renderServerHead->>ssrRenderTags: serialize customized tags
renderServerHead->>ssrRenderTagsTrusted: serialize trusted tags
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/unhead/src/utils/resolve.ts`:
- Around line 99-112: In packages/unhead/src/utils/resolve.ts#L99-L112, update
the property merge loop to process only own source keys and reject __proto__,
constructor, and prototype before assigning to props; preserve the existing
style and class merging behavior. In
packages/unhead/src/server/util/propsToString.ts#L15-L17, always skip keys that
are not own properties of props, while trusted mode may only bypass empty-name
and attribute-name validation. Add a regression covering parsed JSON with a
__proto__ value merged into htmlAttrs.
🪄 Autofix
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 Plus
Run ID: 3d91e5b9-c047-400a-a7fb-6b675b959874
📒 Files selected for processing (8)
bench/resolve-tags.bench.tspackages/unhead/src/server/renderSSRHead.tspackages/unhead/src/server/util/propsToString.tspackages/unhead/src/server/util/ssrRenderTags.tspackages/unhead/src/server/util/tagToString.tspackages/unhead/src/utils/resolve.tspackages/unhead/test/unit/hooks/mutating-hooks.test.tspackages/unhead/test/unit/server/deduping.test.ts
Contributor
📦 Bundle Size
All bundles (14)
📦 Runtime Dependencies✅ No runtime dependency changes All packages (10)
Skipped optional dependencies (18)
Production dependencies only. Peer dependencies and Unhead workspace packages are excluded. Skipped optional dependencies are unavailable on the CI platform. ⚡ Performance (directional)🟢 4 faster
All benchmarks (28)
Baseline: main @ 94214ab · 2026-08-12 · gzipped is the headline size metric · perf is directional (shared-runner, gated) |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/unhead/test/unit/server/prototype-pollution.test.ts (2)
105-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSelect the test entry by its data, not by hook call count.
The hook at Line [110] depends on the second
entries:normalizecall. If the lifecycle adds another normalization pass, the payload can target the wrong entry or no entry. Select thehtmlAttrstag whoseclassissecond.Proposed test fix
- let normalizeCount = 0 let inheritedOnload: unknown @@ - if (++normalizeCount === 2) - tags[0].props = JSON.parse('{"lang":"en","__proto__":{"onload":"alert(1)"}}') + const htmlAttrs = tags.find(tag => tag.tag === 'htmlAttrs') + if (htmlAttrs?.props?.class === 'second') + htmlAttrs.props = JSON.parse('{"lang":"en","__proto__":{"onload":"alert(1)"}}')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/unhead/test/unit/server/prototype-pollution.test.ts` around lines 105 - 111, Update the entries:normalize hook to locate the htmlAttrs tag whose class is "second" and assign the prototype-pollution payload to that tag, instead of relying on normalizeCount reaching 2. Remove the call-count dependency while preserving the existing payload and test behavior.
121-124: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that safe attributes survive filtering.
The payload at Line [111] includes valid
langdata, but the test only checks thatonloadis absent. An implementation that drops the completepropsobject would pass. Assert that the rendered output keepslang="en".Proposed test assertion
expect(inheritedOnload).toBeUndefined() + expect(result.htmlAttrs).toContain('lang="en"') expect(result.htmlAttrs).not.toContain('onload')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/unhead/test/unit/server/prototype-pollution.test.ts` around lines 121 - 124, Update the assertions in the prototype-pollution test around renderSSRHead to verify that result.htmlAttrs preserves the valid lang="en" attribute, while retaining the existing checks that inherited onload data is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/unhead/test/unit/server/prototype-pollution.test.ts`:
- Around line 105-111: Update the entries:normalize hook to locate the htmlAttrs
tag whose class is "second" and assign the prototype-pollution payload to that
tag, instead of relying on normalizeCount reaching 2. Remove the call-count
dependency while preserving the existing payload and test behavior.
- Around line 121-124: Update the assertions in the prototype-pollution test
around renderSSRHead to verify that result.htmlAttrs preserves the valid
lang="en" attribute, while retaining the existing checks that inherited onload
data is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f71e190-48e6-4f79-b3f0-5c806f845fdb
📒 Files selected for processing (3)
packages/unhead/src/server/util/propsToString.tspackages/unhead/src/utils/resolve.tspackages/unhead/test/unit/server/prototype-pollution.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/unhead/src/server/util/propsToString.ts
- packages/unhead/src/utils/resolve.ts
This reverts commit 663c20b.
…aths # Conflicts: # packages/unhead/src/server/createHead.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/schema-org/test/ssr/cache.test.ts (1)
5-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test that asserts a cache hit.
All five tests assert invalidation or non-caching. They also pass when the cache never activates. That leaves the optimization itself untested and hides a silently disabled cache.
Add a test with static entries that renders twice and asserts the graph is resolved only once, for example by counting invocations of a resolver attached to a built-in node, or by spying on
resolveGraph.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/schema-org/test/ssr/cache.test.ts` around lines 5 - 93, Add a dedicated cache-hit test alongside the existing schema.org render cache tests using static schema entries and a counting resolver or resolveGraph spy. Render the same head twice, assert the resolver or graph-resolution function runs only once, and keep the test focused on proving the second render reuses cached output.packages/schema-org/src/plugin.ts (1)
25-39: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffExpose hook inspection through a supported API.
TemplateParamsPluginandFlatMetaPluginmark deterministic hooks. Unhead does not register them by default, and dynamic hooks such asPromisesPluginmust remain unmarked. Replace directhead.hooks._hooksaccess with a public unhead helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/schema-org/src/plugin.ts` around lines 25 - 39, Update hooksAreCacheable to inspect registered hooks through Unhead’s supported public hook-inspection helper instead of accessing the private head.hooks._hooks structure. Preserve the existing TagMutationHook filtering and StaticHook/ownHooks checks, ensuring deterministic plugin hooks remain cacheable while dynamic hooks remain unmarked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/schema-org/src/plugin.ts`:
- Around line 156-160: Move the per-render state variables cache, entries, and
reuseGraph from the UnheadSchemaOrg factory closure into the defineHeadPlugin
callback that receives each head instance, keeping them initialized identically.
Ensure all entries:resolve and tags:resolve logic uses this callback-local state
so concurrent heads cannot share cached graph or reuse decisions; leave shared
factory-level state such as resolvedMeta unchanged.
In `@packages/unhead/src/utils/normalize.ts`:
- Around line 100-108: Update the value resolution in the normalization flow
around walkResolver so on* function handlers are preserved instead of executed
during normalization, allowing the server event-handler branch to pass the
original function to resolve and create the dataset marker. Add a regression
test covering SSR normalization with an on* handler that throws, asserting the
handler is not invoked and the marker is produced.
---
Nitpick comments:
In `@packages/schema-org/src/plugin.ts`:
- Around line 25-39: Update hooksAreCacheable to inspect registered hooks
through Unhead’s supported public hook-inspection helper instead of accessing
the private head.hooks._hooks structure. Preserve the existing TagMutationHook
filtering and StaticHook/ownHooks checks, ensuring deterministic plugin hooks
remain cacheable while dynamic hooks remain unmarked.
In `@packages/schema-org/test/ssr/cache.test.ts`:
- Around line 5-93: Add a dedicated cache-hit test alongside the existing
schema.org render cache tests using static schema entries and a counting
resolver or resolveGraph spy. Render the same head twice, assert the resolver or
graph-resolution function runs only once, and keep the test focused on proving
the second render reuses cached output.
🪄 Autofix
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 Plus
Run ID: 601a114b-558d-41e4-8abb-acf1f5961a8b
📒 Files selected for processing (19)
.github/workflows/bundle-size.ymlpackages/schema-org/src/core/define.tspackages/schema-org/src/plugin.tspackages/schema-org/src/runtime.tspackages/schema-org/test/ssr/cache.test.tspackages/unhead/src/client/createHead.tspackages/unhead/src/composables.tspackages/unhead/src/plugins/flatMeta.tspackages/unhead/src/plugins/templateParams.tspackages/unhead/src/server/createHead.tspackages/unhead/src/server/renderSSRHead.tspackages/unhead/src/types/head.tspackages/unhead/src/utils/hooks.tspackages/unhead/src/utils/meta.tspackages/unhead/src/utils/normalize.tspackages/unhead/src/utils/resolve.tspackages/unhead/test/unit/hooks/mutating-hooks.test.tspackages/unhead/test/unit/server/eventHandlers.test.tspackages/unhead/test/unit/server/prototype-pollution.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/unhead/test/unit/server/prototype-pollution.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/unhead/test/unit/templateParams.test.ts (1)
5-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for malformed encoded input.
The implementation has a fallback for malformed URI sequences, but the tests cover only valid
%20input and ordinary template tokens. Add a case that preserves malformed input while still resolving%siteName. This protects the behavior used bypackages/schema-org/src/plugin.ts, Line 309.Suggested regression test
it('resolves tokens that are not encoded percent sequences', () => { expect(processTemplateParams('%s %separator %siteName', { pageTitle: 'Page', siteName: 'Unhead', }, '|')).toBe('Page | Unhead') }) + + it('preserves malformed encoded input while resolving template tokens', () => { + expect(processTemplateParams('https://example.com/%E0%A4%A/%siteName', { + siteName: 'Unhead', + })).toBe('https://example.com/%E0%A4%A/Unhead') + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/unhead/test/unit/templateParams.test.ts` around lines 5 - 15, Add a regression test alongside the existing processTemplateParams cases for a malformed percent-encoded sequence, asserting the malformed input is preserved while the %siteName token still resolves. Keep the expectation focused on the fallback behavior used by processTemplateParams.packages/unhead/src/utils/templateParams.ts (1)
7-7: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winNarrow
ENCODED_PERCENT_REbefore callingdecodeURI.
%description,%canonical, and%datematch%de,%ca, and%da, sodecodeURIthrowsURIErrorfor valid template tokens. This adds exception overhead on the SSR path. Resolve template tokens before decoding or use a detector that excludes valid token names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/unhead/src/utils/templateParams.ts` at line 7, Update ENCODED_PERCENT_RE and its use in the template-parameter decoding flow so valid tokens such as %description, %canonical, and %date are resolved before decodeURI or excluded from encoded-percent detection. Ensure decodeURI is only called for actual percent-encoded byte sequences, avoiding URIError and unnecessary exception handling for valid template tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/unhead/src/utils/templateParams.ts`:
- Line 7: Update ENCODED_PERCENT_RE and its use in the template-parameter
decoding flow so valid tokens such as %description, %canonical, and %date are
resolved before decodeURI or excluded from encoded-percent detection. Ensure
decodeURI is only called for actual percent-encoded byte sequences, avoiding
URIError and unnecessary exception handling for valid template tokens.
In `@packages/unhead/test/unit/templateParams.test.ts`:
- Around line 5-15: Add a regression test alongside the existing
processTemplateParams cases for a malformed percent-encoded sequence, asserting
the malformed input is preserved while the %siteName token still resolves. Keep
the expectation focused on the fallback behavior used by processTemplateParams.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e177d49-14be-46ed-93aa-19b05bc5cd78
📒 Files selected for processing (11)
bench/bundle/perf-report.test.tsbench/bundle/perf-report.tsbench/perf-ci.mjspackages/schema-org/src/plugin.tspackages/schema-org/test/ssr/cache.test.tspackages/unhead/src/composables.tspackages/unhead/src/server/createHead.tspackages/unhead/src/utils/normalize.tspackages/unhead/src/utils/templateParams.tspackages/unhead/test/unit/server/eventHandlers.test.tspackages/unhead/test/unit/templateParams.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/unhead/src/composables.ts
- packages/unhead/src/server/createHead.ts
- packages/unhead/src/utils/normalize.ts
- packages/schema-org/test/ssr/cache.test.ts
Collaborator
Author
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
❓ Type of change
📚 Description
This removes work from each SSR pipeline stage:
resolvedTagskeep full validation.Local shipped-output comparisons against current main show:
All 1,980 tests pass, with 8 skipped. Lint, typecheck, full build, and local performance reporting pass.
Summary by CodeRabbit
Bug Fixes
Performance