feat(validate): add performance validation rules#691
Merged
Conversation
…nfig
Add 8 performance hint rules inspired by webperf-snippets: preload-fetchpriority-conflict,
too-many-preloads, too-many-preconnects, redundant-dns-prefetch, preload-async-defer-conflict,
prefetch-preload-conflict, inline-style-size, inline-script-size.
Add type-safe ESLint-style rule configuration with per-rule options support via
discriminated union types (e.g. `['warn', { max: 10 }]` tuples).
📝 WalkthroughWalkthroughThis PR extends the head validation plugin with eight new performance-related validation rules, adds type-safe per-rule configuration options, introduces helper functions to resolve configurations, and includes comprehensive test coverage for all new rules. Changes
Sequence DiagramsequenceDiagram
participant Config as Configuration Input
participant Resolver as Option Resolver
participant Checks as Validation Checks
participant Report as Report Output
Config->>Resolver: RulesConfig with [Severity, Options]?
Resolver->>Resolver: resolveSeverity()
Resolver->>Resolver: resolveOptions() with defaults
Resolver->>Checks: Severity + Resolved Options
Checks->>Checks: Execute check with options<br/>(e.g., maxKB, max threshold)
alt Rule Violation Detected
Checks->>Report: Report with severity
else Rule Passes
Checks->>Report: No report
end
Report->>Report: Return validation results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Bundle Size Analysis
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/head/1.guides/plugins/validate.md (1)
62-64:⚠️ Potential issue | 🟡 MinorDocumentation shows outdated type signature.
The interface definition shows
rules?: Partial<Record<string, 'warn' | 'info' | 'off'>>but the actual implementation usesRulesConfigwhich supports the ESLint-style[severity, options]tuple for rules with configurable options.📝 Suggested fix to align with implementation
/** - * Configure rule severity. Set to 'off' to disable, or 'warn'/'info' to override. + * Configure rule severity and options. Accepts a severity string or an ESLint-style + * `[severity, options]` tuple for rules that support configuration. */ - rules?: Partial<Record<string, 'warn' | 'info' | 'off'>> + rules?: RulesConfig🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/head/1.guides/plugins/validate.md` around lines 62 - 64, The docs show an outdated type for the rules property; update the documentation to reflect the actual implementation by replacing the current signature "rules?: Partial<Record<string, 'warn' | 'info' | 'off'>>" with the correct type that uses RulesConfig (i.e., allow ESLint-style rule entries like [severity, options] and tuples), and mention that the configuration accepts severity strings or [severity, options] tuples consistent with the RulesConfig used in the codebase.
🧹 Nitpick comments (1)
packages/unhead/src/plugins/validate.ts (1)
502-516: Consider normalizing origin URLs for comparison.The redundant dns-prefetch check uses exact string matching. Origins with subtle differences (e.g., trailing slash, different casing) won't match. This could lead to false negatives in edge cases.
🔧 Optional: Normalize origins before comparison
+function normalizeOrigin(href: string): string { + try { + const url = new URL(href) + return url.origin + } catch { + return href.toLowerCase().replace(/\/$/, '') + } +} + // Redundant dns-prefetch when preconnect exists for same origin const preconnectOrigins = new Set<string>() const dnsPrefetchTags: HeadTag[] = [] for (const tag of tags) { if (tag.tag === 'link' && tag.props.href) { if (tag.props.rel === 'preconnect') - preconnectOrigins.add(tag.props.href) + preconnectOrigins.add(normalizeOrigin(tag.props.href)) else if (tag.props.rel === 'dns-prefetch') dnsPrefetchTags.push(tag) } } for (const tag of dnsPrefetchTags) { - if (preconnectOrigins.has(tag.props.href)) + if (preconnectOrigins.has(normalizeOrigin(tag.props.href))) report('redundant-dns-prefetch', `dns-prefetch for "${tag.props.href}" is redundant — preconnect already includes DNS resolution.`, 'info', tag) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/unhead/src/plugins/validate.ts` around lines 502 - 516, The redundancy check currently compares link href strings verbatim (using preconnectOrigins.has(tag.props.href)), which misses semantically equal origins (e.g., differing case or trailing slashes); update the logic in the validate plugin around the preconnectOrigins set and dnsPrefetchTags loop to normalize origins before storing/checking: for each link href (in the block that populates preconnectOrigins and later when iterating dnsPrefetchTags) compute a canonical origin (e.g., use the URL constructor to extract URL.origin or otherwise lower-case and trim trailing slashes, with a try/catch for invalid/relative hrefs and a sensible fallback) and use that normalized origin as the Set key and for the has() check so report('redundant-dns-prefetch', ...) fires correctly for equivalent origins.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/head/1.guides/plugins/validate.md`:
- Around line 62-64: The docs show an outdated type for the rules property;
update the documentation to reflect the actual implementation by replacing the
current signature "rules?: Partial<Record<string, 'warn' | 'info' | 'off'>>"
with the correct type that uses RulesConfig (i.e., allow ESLint-style rule
entries like [severity, options] and tuples), and mention that the configuration
accepts severity strings or [severity, options] tuples consistent with the
RulesConfig used in the codebase.
---
Nitpick comments:
In `@packages/unhead/src/plugins/validate.ts`:
- Around line 502-516: The redundancy check currently compares link href strings
verbatim (using preconnectOrigins.has(tag.props.href)), which misses
semantically equal origins (e.g., differing case or trailing slashes); update
the logic in the validate plugin around the preconnectOrigins set and
dnsPrefetchTags loop to normalize origins before storing/checking: for each link
href (in the block that populates preconnectOrigins and later when iterating
dnsPrefetchTags) compute a canonical origin (e.g., use the URL constructor to
extract URL.origin or otherwise lower-case and trim trailing slashes, with a
try/catch for invalid/relative hrefs and a sensible fallback) and use that
normalized origin as the Set key and for the has() check so
report('redundant-dns-prefetch', ...) fires correctly for equivalent origins.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 51ebfff3-5f6b-4357-af10-1363f0bbf582
📒 Files selected for processing (4)
docs/head/1.guides/plugins/validate.mdpackages/unhead/src/plugins/index.tspackages/unhead/src/plugins/validate.tspackages/unhead/test/unit/plugins/validate.test.ts
harlan-zw
changed the title
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.
🔗 Linked issue
N/A
❓ Type of change
📚 Description
Adds 8 performance validation rules to the ValidatePlugin, inspired by webperf-snippets. These catch common performance anti-patterns in head tags:
preload-fetchpriority-conflict— contradictory preload + low fetchprioritytoo-many-preloads— more than 6 preloads competing for bandwidthtoo-many-preconnects— more than 4 preconnects saturating connectionsredundant-dns-prefetch— dns-prefetch redundant when preconnect existspreload-async-defer-conflict— preloaded script with async/deferprefetch-preload-conflict— same resource both preloaded and prefetchedinline-style-size— inline style exceeds 14KB critical CSS budgetinline-script-size— inline script exceeds 2KB cacheability thresholdAlso introduces ESLint-style type-safe rule configuration. Rules with options accept
[severity, options]tuples with per-rule typed options:Summary by CodeRabbit
New Features
Documentation