feat: PERF009 — heavy dependency import (Bundle slice) - #74
Conversation
Flags an import from a well-known heavy / non-tree-shakeable package (lodash, moment), matched by exact specifier so subpath imports (lodash/debounce) pass. Reused the component scan; ComponentFacts gains `imports` (module specifiers from the instance + module scripts). componentRule's category widens to 'performance'; reported under the performance category (info). Docs (en+ja), changeset, spec, tests (import capture + rule). pnpm -r test 536 green; typecheck, lint, docs build (107 pages) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds PERF009, a performance rule flagging exact imports of heavy, non-tree-shakeable packages ( ChangesPERF009 Heavy Import Rule
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Parser
participant Facts as ComponentFacts
participant Rule as perf009HeavyImport
participant Report as Findings Report
CLI->>Facts: collect imports from script blocks
Facts->>Rule: provide imports[] via RuleContext
Rule->>Rule: match against HEAVY_PACKAGES allowlist
alt heavy package matched
Rule->>Report: emit performance/info finding with recommendation
else no match
Rule-->>Report: no finding
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/component.ts (1)
43-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
importslacks location info, forcing PERF009 to reportline: 0.Unlike
htmlTags/javascriptUrls(which useSourceSpan[]),importsis plainstring[]. As a result,perf009HeavyImport'sbad()hardcodesline: 0for every finding (seepackages/core/src/rules/performance/perf009-heavy-import.ts), so all PERF009 diagnostics point to the top of the file instead of the actual import line — this will look like a bug in generated reports.Consider changing
importsto carry the source line (e.g.,{ specifier: string; line: number }[]) socollectImportSourcesinparse.tscan populate it and the rule can emit accurate locations.♻️ Sketch of the type change
- /** Module specifiers of every `import` in the instance + module scripts (Bundle PERF009). */ - imports: string[]; + /** Every `import` in the instance + module scripts, with source line (Bundle PERF009). */ + imports: { specifier: string; line: number }[];🤖 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/core/src/component.ts` around lines 43 - 44, The `imports` field in `Component` only stores specifiers, so `perf009HeavyImport` cannot report real source locations and falls back to `line: 0`. Update the `Component` type to carry line information for imports (similar to `htmlTags` and `javascriptUrls`), then adjust `collectImportSources` in `parse.ts` to populate that data from each import’s source span. Finally, update `perf009HeavyImport.bad()` in `perf009-heavy-import.ts` to read the stored line from `imports` instead of hardcoding `0`.packages/core/src/rules/performance/perf009-heavy-import.ts (1)
7-10: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard against prototype-chain lookups in
HEAVY_PACKAGES.
src in HEAVY_PACKAGESchecks the prototype chain, so a component importing a package literally namedconstructor,toString, orhasOwnPropertywould incorrectly match, andHEAVY_PACKAGES[src]would resolve to an inheritedObject.prototypevalue in the message.🛡️ Proposed fix
applies: (c) => c.imports.length > 0, bad: (c) => c.imports - .filter((src) => src in HEAVY_PACKAGES) + .filter((src) => Object.hasOwn(HEAVY_PACKAGES, src)) .map((src) => ({ line: 0, message: `Heavy import "${src}" — ${HEAVY_PACKAGES[src]}` })) });Also applies to: 22-25
🤖 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/core/src/rules/performance/perf009-heavy-import.ts` around lines 7 - 10, `HEAVY_PACKAGES` lookup in `perf009-heavy-import.ts` should not rely on prototype-chain membership checks. Update the logic that decides whether a package is heavy (the `HEAVY_PACKAGES` map lookup used with `src`) to use an own-property check or a null-prototype map so imports like `constructor` or `toString` cannot match inherited `Object.prototype` keys. Keep the warning message generation in sync with the safe lookup in the same rule implementation.
🤖 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/core/src/component.ts`:
- Around line 43-44: The `imports` field in `Component` only stores specifiers,
so `perf009HeavyImport` cannot report real source locations and falls back to
`line: 0`. Update the `Component` type to carry line information for imports
(similar to `htmlTags` and `javascriptUrls`), then adjust `collectImportSources`
in `parse.ts` to populate that data from each import’s source span. Finally,
update `perf009HeavyImport.bad()` in `perf009-heavy-import.ts` to read the
stored line from `imports` instead of hardcoding `0`.
In `@packages/core/src/rules/performance/perf009-heavy-import.ts`:
- Around line 7-10: `HEAVY_PACKAGES` lookup in `perf009-heavy-import.ts` should
not rely on prototype-chain membership checks. Update the logic that decides
whether a package is heavy (the `HEAVY_PACKAGES` map lookup used with `src`) to
use an own-property check or a null-prototype map so imports like `constructor`
or `toString` cannot match inherited `Object.prototype` keys. Keep the warning
message generation in sync with the safe lookup in the same rule implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f206f6c5-c419-4414-b500-b0e96239ac87
📒 Files selected for processing (16)
.changeset/bundle-heavy-imports.mddocs/src/content/docs/ja/rules/perf009.mddocs/src/content/docs/rules/perf009.mddocs/superpowers/specs/2026-07-01-bundle-heavy-imports-design.mdpackages/cli/src/providers/source/components.tspackages/cli/src/providers/source/parse.tspackages/cli/test/parse-component-facts.test.tspackages/core/src/component.tspackages/core/src/index.tspackages/core/src/rules/component-rule.tspackages/core/src/rules/index.tspackages/core/src/rules/performance/perf009-heavy-import.tspackages/core/test/architecture-rules.test.tspackages/core/test/bundle-rules.test.tspackages/core/test/correctness-rules.test.tspackages/core/test/security-rules.test.ts
There was a problem hiding this comment.
Pull request overview
Adds a new component-scoped performance rule (PERF009) to flag bare imports of known heavy, non-tree-shakeable dependencies (currently lodash and moment) using the existing CLI/static component-facts channel (ctx.components). This expands ComponentFacts to include collected import specifiers from both instance and module scripts, enabling deterministic bundle/perf hygiene checks aligned with the roadmap in #69.
Changes:
- Introduces PERF009 rule (info, performance) that flags exact-match heavy dependency imports and suggests lighter/subpath alternatives.
- Extends CLI component parsing to collect
importspecifiers from both<script>and<script module>intoComponentFacts.imports. - Adds tests, docs (EN/JA), a design spec, and a changeset for the new rule.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/test/security-rules.test.ts | Updates ComponentFacts test helper to include new imports field. |
| packages/core/test/correctness-rules.test.ts | Updates ComponentFacts test helper to include new imports field. |
| packages/core/test/architecture-rules.test.ts | Updates ComponentFacts test helper to include new imports field. |
| packages/core/test/bundle-rules.test.ts | Adds new PERF009 unit tests for pass/fail/no-op behaviors. |
| packages/core/src/rules/performance/perf009-heavy-import.ts | Implements PERF009 heavy import rule using component facts. |
| packages/core/src/rules/index.ts | Registers/exports PERF009 in the core rules list and exports. |
| packages/core/src/rules/component-rule.ts | Extends component-rule categories to allow performance-scoped component rules. |
| packages/core/src/index.ts | Re-exports PERF009 from the core entrypoint. |
| packages/core/src/component.ts | Adds imports: string[] to ComponentFacts. |
| packages/cli/test/parse-component-facts.test.ts | Adds parser tests for collecting imports from instance/module scripts and preserving subpaths. |
| packages/cli/src/providers/source/parse.ts | Collects ImportDeclaration.source.value into imports during static parsing. |
| packages/cli/src/providers/source/components.ts | Ensures parse-failure fallback ComponentFacts includes imports: []. |
| docs/superpowers/specs/2026-07-01-bundle-heavy-imports-design.md | Adds design spec for PERF009 and import-fact capture. |
| docs/src/content/docs/rules/perf009.md | Adds EN rule documentation page for PERF009. |
| docs/src/content/docs/ja/rules/perf009.md | Adds JA rule documentation page for PERF009. |
| .changeset/bundle-heavy-imports.md | Declares release notes/version bumps for PERF009 and ComponentFacts.imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…review) Match the heavy-package allowlist with Object.hasOwn (not `in`), so inherited keys like `toString`/`constructor` never false-match; dedupe per specifier so the same package imported in both <script module> and <script> isn't double-penalized. Tests added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Bundle/perf slice of #69 — flag imports of well-known heavy / non-tree-shakeable packages that bloat the bundle (a common "AI wrote
import _ from 'lodash'" mistake). Allowlist-precise (exact specifier match), reusing the component-body scan (ctx.components, CLI/static).New rule
infoFlags
import … from 'lodash'/'moment'. Matched exactly — a subpath import (lodash/debounce) is the fix, so it passes. Reported under the existing performance category.What
ComponentFactsgainsimports— module specifiers of everyimportin the instance and module<script>(ESTreeImportDeclaration.source.value).componentRule'sComponentCategorywidens to include'performance'(a component-scoped perf rule).Tests / docs
importsfrom instance + module scripts; subpath specifiers verbatim.lodash/moment; passeslodash/debounce/date-fns/ no imports; no-op whenctx.componentsunset.pnpm -r test(536: core 241 / vite 76 / cli 210 / mcp 9),pnpm -r typecheck,pnpm lint,pnpm --filter docs build(107 pages) — all green.Out of scope
Real byte-size / bundle analysis (needs a bundler), configurable allowlist, and
import * asnamespace heuristics — allowlist only for now.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests