feat(core): add architecture/route-component-import - #335
Conversation
… gaps - Memoize `routeEntryImports` per component in `architecture/route-component-import` so `applies`/`bad` share one resolution pass instead of resolving every import specifier twice per analysis (measured ~27-30% reduction in an isolated per-rule microbenchmark; the whole-project bench is too noisy to show a few-ms change). - Add `configuration.mdx` (en + ja) coverage for this rule: it belongs in the "rules that take options" list (`exemptImporters`) and the "Import aliases" list, both of which had omitted it. - Point the rule's test at the package barrel (`../src/index.js`) like its siblings, so a missed registration in the fourth site fails a test. - Add an end-to-end test that parses real Svelte source (a value import and a type-only import of a route entry) through `parseComponentFacts` into the rule, pinning the Task 1/Task 2 seam. - Update two "why this is exported" comments (`routeGlobToRegExp`, `resolveRepoLocalPath`) to name this rule as a second consumer.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds the default-on ChangesRoute component import rule
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SvelteComponent
participant componentParse
participant componentRule
participant routeComponentImport
participant resolveRepoLocalPath
SvelteComponent->>componentParse: expose importSpans
componentRule->>routeComponentImport: pass ComponentFacts and RuleContext
routeComponentImport->>resolveRepoLocalPath: resolve import source
resolveRepoLocalPath-->>routeComponentImport: return repository-local path
routeComponentImport-->>componentRule: report route-entry diagnostics
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/src/content/docs/rules/index.mdx`:
- Line 27: Replace the hard-coded Architecture rule count in
docs/src/content/docs/rules/index.mdx lines 27-27 with durable non-counted
wording. Apply the equivalent durable Japanese wording in
docs/src/content/docs/ja/rules/index.mdx lines 29-29, removing the fixed count
in both indexes.
In `@docs/superpowers/plans/2026-07-31-route-component-import.md`:
- Around line 576-587: Fix the Markdown fence around the rule-page example in
the surrounding documentation: remove the premature closing fence before “## Not
reported” and place the closing fence after the entire “Not reported” section,
ensuring the example remains inside one properly matched fence.
In `@packages/core/src/rules/architecture/route-component-import.ts`:
- Around line 61-66: Scope cachedRouteEntryImports to the current evaluation
context instead of sharing results process-wide: remove routeEntryImportsCache
or key it with ctx.project.kitAliases (and the relevant context identity)
alongside ComponentFacts. Ensure each check() recomputes or retrieves only
imports resolved for the current aliases, preventing stale targets across
evaluations.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 341bfa9e-70d3-40f0-959d-470f1535c5a0
📒 Files selected for processing (21)
.changeset/route-component-import.mddocs/src/content/docs/guides/(setup)/configuration.mdxdocs/src/content/docs/ja/guides/(setup)/configuration.mdxdocs/src/content/docs/ja/rules/architecture/index.mdxdocs/src/content/docs/ja/rules/architecture/route-component-import.mddocs/src/content/docs/ja/rules/index.mdxdocs/src/content/docs/rules/architecture/index.mdxdocs/src/content/docs/rules/architecture/route-component-import.mddocs/src/content/docs/rules/index.mdxdocs/superpowers/plans/2026-07-31-route-component-import.mddocs/superpowers/specs/2026-07-30-route-component-import-design.mdpackages/core/src/component-parse.tspackages/core/src/component.tspackages/core/src/config-apply.tspackages/core/src/index.tspackages/core/src/kit-module-parse.tspackages/core/src/rules/architecture/route-component-import.tspackages/core/src/rules/component-rule.tspackages/core/src/rules/index.tspackages/core/test/component-parse.test.tspackages/core/test/route-component-import.test.ts
The per-component cache in architecture/route-component-import was keyed only on the ComponentFacts WeakMap identity, but the memoized computation also depends on ctx.project.kitAliases. A caller reusing the same ComponentFacts across two check() calls with different alias configuration got back the first call's resolved targets. Store the aliases reference alongside the cached result and recompute when it differs (reference equality is correct: a fresh analysis always rebuilds the alias array). Added a regression test that reuses one ComponentFacts object across two check() calls with different kitAliases and confirmed it fails without the fix. Also, while reviewing the same PR: - Remove the generated (N rules) / (N 件のルール) suffixes from the rule index cards. They live inside gen-rules-index.mjs's generated block (not hand-written prose as initially assumed), so the fix is in the generator (packages/cli/scripts/rules-index.mjs) rather than the .mdx files directly; regenerated both locales' index pages and re-ran the formatter. - Fix an unmatched Markdown fence in docs/superpowers/plans/2026-07-31-route-component-import.md: the outer fence around the Step 2 rule-page example closed before its "Not reported" section instead of after, and a stray unlabeled fence followed. Closed the outer fence in the right place and normalized one other asymmetric fence pair found while checking the rest of the file.
There was a problem hiding this comment.
Pull request overview
This PR adds a new default-on Architecture rule to @svelte-vitals/core that detects Svelte components importing SvelteKit route entry components (+page.svelte, +layout.svelte, +error.svelte, including @ breakout forms) from within src/routes/, which can render without the data SvelteKit normally provides.
Changes:
- Add
architecture/route-component-import(severity:info), including alias-aware specifier resolution and configurable importer exemptions. - Extend component import facts to mark type-only imports (
import type …and all-inline-typed specifiers) so the rule ignores imports with no runtime binding. - Add tests, documentation (en/ja), rules index regeneration changes, and a changeset for release.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/test/route-component-import.test.ts | New unit tests covering route-entry detection, alias resolution, caching, type-only skips, and exemptions. |
| packages/core/test/component-parse.test.ts | Adds coverage ensuring importSpans marks type-only imports. |
| packages/core/src/rules/index.ts | Registers and re-exports the new Architecture rule in the core rule registry. |
| packages/core/src/rules/component-rule.ts | Extends componentRule callbacks to receive RuleContext for alias-aware rule logic. |
| packages/core/src/rules/architecture/route-component-import.ts | Implements the new architecture/route-component-import rule. |
| packages/core/src/kit-module-parse.ts | Updates doc comment to reflect new internal consumer of resolveRepoLocalPath. |
| packages/core/src/index.ts | Adds the new rule to the public re-export list. |
| packages/core/src/config-apply.ts | Updates doc comment to reflect route-glob compilation reuse by the new rule. |
| packages/core/src/component.ts | Extends ComponentFacts.importSpans to include an optional type?: true marker. |
| packages/core/src/component-parse.ts | Implements detection of type-only import declarations when collecting importSpans. |
| packages/cli/scripts/rules-index.mjs | Removes rule-count text from generated rules index pages (avoids hard-coded counts). |
| docs/superpowers/specs/2026-07-30-route-component-import-design.md | Updates design doc status/blocker notes and documents additional “not reported” cases. |
| docs/superpowers/plans/2026-07-31-route-component-import.md | Adds an implementation plan for the rule and supporting changes. |
| docs/src/content/docs/rules/index.mdx | Updates generated rules listing to include the new rule and remove hard-coded rule counts. |
| docs/src/content/docs/rules/architecture/route-component-import.md | Adds English documentation page for the new rule. |
| docs/src/content/docs/rules/architecture/index.mdx | Updates Architecture rules index to include the new rule. |
| docs/src/content/docs/ja/rules/index.mdx | Updates Japanese rules listing to include the new rule and remove hard-coded rule counts. |
| docs/src/content/docs/ja/rules/architecture/route-component-import.md | Adds Japanese documentation page for the new rule. |
| docs/src/content/docs/ja/rules/architecture/index.mdx | Updates Japanese Architecture rules index to include the new rule. |
| docs/src/content/docs/ja/guides/(setup)/configuration.mdx | Documents the new rule option and adds it to the alias-following rules list (ja). |
| docs/src/content/docs/guides/(setup)/configuration.mdx | Documents the new rule option and adds it to the alias-following rules list (en). |
| .changeset/route-component-import.md | Declares a minor release for packages impacted by adding a default-on rule. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Why
A SvelteKit route entry —
+page.svelte,+layout.svelte,+error.svelte— is written on the assumption that Kit renders it. Kit hands a page itsdataandparams; it hands an error page itspage.errorandpage.status. Imported from somewhere else, the component receives none of that and renders against nothing, or against the importing page's data standing in for its own.The mistake is easy to make and reads as reasonable: another page needs the same markup, the markup already exists in a
+page.svelte, so it gets imported. Nothing in the toolchain objects. The component renders — emptily.What
architecture/route-component-import, atinfo. It resolves each import specifier to a project-relative path, and reports one whose basename is a route-entry name and which sits under the routes directory. Those two conditions are separate on purpose: a file named+page.svelteoutsidesrc/routesis not a route entry, because Kit gives those names meaning only there.This is the first Architecture rule that is on by default. The three that shipped before it assert nothing until configured, so a design error there costs a user nothing; here it reaches everyone who upgrades. Two consequences shaped the design.
The route-entry matcher reproduces Kit's own, not a tidier approximation.
analyze()in@sveltejs/kit/src/core/sync/create_manifest_data/index.jsstrips only the component extension before testing/^\+(?:(page(?:@(.*))?)|(layout(?:@(.*))?)|(error))$/, so the@breakout suffix is unbounded — a layout name may contain dots, and+page@foo.bar.svelteis a real route entry. A narrower[^./]*would silently skip it.exemptImportersships deliberately narrow. Astring-listoption adds to its default and can never shrink it, so the two failure directions are not symmetric:exemptImportersstring-listcannot be shrunkOnly the narrow side leaves a lever, which decides it. Stories, tests and specs are exempt out of the box; configuring the option is an expected step for a project whose satellite convention is its own, and the rule page says so rather than treating it as an edge case.
An exempt importer earns a pass, not silence — its route-entry imports really are fine, which is a true statement worth recording. Putting the exemption into
appliesinstead would call such a file signal-free, which it is not.Supporting changes
ComponentFacts.importSpansgainstype?: true— an import that contributes no runtime value binding. It coversimport type …and a declaration whose every specifier is inline-typed; a specifier-less side-effect import stays unmarked, because the module really is loaded. Without it the rule would reportimport type P from './+page.svelte', which renders nothing.componentRulehandsapplies/badtheRuleContext, which its siblingkitModuleRulealready did. The rule needsctx.project.kitAliasesto resolve a specifier through a project's declared aliases (feat: resolve import specifiers through a project's declared SvelteKit aliases #330); without it the rule would be blind to exactly the imports it was measured to need. Purely additive — all 23 existing callers declare fewer parameters and are unaffected.Measured reach, recorded rather than assumed
The design was measured against a real monorepo of several SvelteKit apps on a convention-compliant branch. No route entry was imported anywhere, by any file type — the search pattern was validated first against a synthetic file carrying a relative import, an alias import with an
@breakout, and a type-only import, all three of which matched. So the rule reports nothing and misses nothing on that tree. The built-in exempt list also covered only a minority of that tree's satellite files, which is why configuration is documented as expected rather than exceptional.Not reported
Dynamic
import()(not an import declaration); an import made from a plain.ts/.jsfile, or from a.svelte.ts/.svelte.jsrunes module (the parser leaves those files' import spans empty); a type-only import; and a project whose routes live outsidesrc/routes. The first is a genuine gap; the second is load-bearing — it is why the exempt list can be three entries long instead of an open-ended guess at every project's test-file convention.Verification
core1119,cli779,vite205,mcp25 tests pass; typecheck clean in all four packages; lint and format clean. Six independent mutations are each caught by a test — including the dotted@suffix, the type-only skip, and moving the exemption intoapplies— and one test carries a route entry from real source text through the parser into the rule, so the seam between the fact and its consumer is covered rather than assumed.Design:
docs/superpowers/specs/2026-07-30-route-component-import-design.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests