Skip to content

fix: import protection robustness#6746

Merged
schiller-manuel merged 3 commits intomainfrom
import-protection-robustness
Feb 23, 2026
Merged

fix: import protection robustness#6746
schiller-manuel merged 3 commits intomainfrom
import-protection-robustness

Conversation

@schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Feb 23, 2026

Summary by CodeRabbit

  • Documentation

    • Added guidance on false positives and differences between dev vs. build behavior for import protection; updated plugin docs.
  • New Features

    • Two demo routes added to showcase component server-leak and barrel re-export scenarios.
    • onViolation callback now supports async handlers (may return a Promise).
  • Improvements

    • Build-mode verification enhanced to track and deduplicate deferred violations to reduce false positives.
  • Tests

    • Expanded end-to-end tests for barrel re-exports, marker-protected imports, and component server-leak cases.

@github-actions github-actions bot added documentation Everything documentation related package: start-plugin-core labels Feb 23, 2026
@nx-cloud
Copy link

nx-cloud bot commented Feb 23, 2026

View your CI Pipeline Execution ↗ for commit 755a654

Command Status Duration Result
nx run tanstack-router-e2e-bundle-size:build --... ✅ Succeeded 1m 22s View ↗

☁️ Nx Cloud last updated this comment at 2026-02-23 20:42:37 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 23, 2026

📝 Walkthrough

Walkthrough

Adds build-time deferred violation handling to import-protection: the plugin emits per-violation build mock modules, records deferredBuildViolations, and verifies tree-shaking in generateBundle to report only real (non-tree-shaken) violations. Also adds e2e tests, fixtures, docs, types, virtual-module constants, and async-capable onViolation handling.

Changes

Cohort / File(s) Summary
Docs
docs/start/framework/react/guide/import-protection.md
Added "False Positives: Dev vs Build" section and updated examples and API signatures to allow async-capable onViolation (Promise return).
E2E test setup
e2e/react-start/import-protection/tests/error-mode.setup.ts, e2e/react-start/import-protection/tests/violations.setup.ts
Registered new routes so test runner visits /component-server-leak and /barrel-false-positive.
E2E routes & UI
e2e/react-start/import-protection/src/routes/__root.tsx, e2e/react-start/import-protection/src/routes/component-server-leak.tsx, e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx
Added two route components and navigation links: component-server-leak (reads server-only secret) and barrel-false-positive (consumes barrel re-exports).
E2E violation fixtures
e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts, .../foo.ts, .../index.ts, .../shared.ts, e2e/react-start/import-protection/src/violations/db-credentials.server.ts
Added barrel re-export test modules (server db, marker foo, shared client-safe exports, barrel index) and a server-only component secret module.
E2E specs
e2e/react-start/import-protection/tests/import-protection.spec.ts
Added tests covering component-level server leaks, barrel re-export false-positive scenarios, and verification across build/dev modes.
Plugin internals doc
packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md
New internal design doc describing architecture, violation types, deferral logic, dev vs build strategies, and E2E testing approach.
Plugin core
packages/start-plugin-core/src/import-protection-plugin/plugin.ts
Introduced DeferredBuildViolation and env.deferredBuildViolations, per-violation build mockModuleIds, resolve/load handling for build-mode mocks, async-capable onViolation, and a generateBundle hook to verify tree-shaking and surface only real violations.
Virtual modules
packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts
Added MOCK_BUILD_PREFIX and RESOLVED_MOCK_BUILD_PREFIX constants for per-violation build mock IDs.
Schema / Types
packages/start-plugin-core/src/schema.ts
Expanded onViolation schema/type to accept `Promise<boolean

Sequence Diagram(s)

sequenceDiagram
    participant Runner as "Dev/Build Runner"
    participant Resolver as "plugin.resolveId/transform"
    participant Loader as "plugin.load/transform-cache"
    participant Bundler as "Vite Bundler"
    participant Generator as "plugin.generateBundle"
    participant Reporter as "Violation Reporter"

    Runner->>Resolver: encounter import (specifier, importer)
    Resolver->>Resolver: detect potential import-protection violation
    alt Build mode
        Resolver->>Resolver: create DeferredBuildViolation with unique mockModuleId
        Resolver->>Loader: return resolved mockModuleId to satisfy import
        Runner->>Bundler: include mock module in bundle graph
        Note over Resolver,Generator: deferredBuildViolations recorded in EnvState
    else Dev mode
        Resolver->>Reporter: report or defer violation immediately (dev path)
        Resolver->>Loader: return dev-time mock or original behavior
    end

    Bundler->>Generator: after bundle generation, invoke generateBundle
    Generator->>Generator: collect emitted module IDs and map mockModuleIds
    alt Mock survived (real violation)
        Generator->>Reporter: emit build-time warning/error for real violation
    else Mock tree-shaken (false positive)
        Generator->>Reporter: no report for this deferred violation
    end

    Reporter-->>Runner: final build status (warnings/errors)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • tannerlinsley
  • brenelz

Poem

🐰 I nudged each mock with tiny paws,
I hid the leaks and checked the laws.
If tree-shake keeps the mock away,
No warning hops to light the day. 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix: import protection robustness' is vague and does not clearly convey the specific changes made in the pull request. Consider using a more descriptive title that highlights the main change, such as 'feat: add async support for onViolation callback and barrel re-export detection' or 'feat: improve import protection with promise-based callbacks and false-positive handling'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch import-protection-robustness

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

@github-actions
Copy link

github-actions bot commented Feb 23, 2026

Bundle Size Benchmarks

  • Commit: c855d6131e58
  • Measured at: 2026-02-23T20:34:09.176Z
  • Baseline source: history:343f8a8ff9ae
  • Dashboard: bundle-size history
Scenario Current (gzip) Delta vs baseline Raw Brotli Trend
react-router.minimal 86.53 KiB 0 B (0.00%) 272.09 KiB 75.11 KiB ▇▇▇▇▇██▆▆▆▁
react-router.full 89.55 KiB 0 B (0.00%) 282.42 KiB 77.86 KiB ███████▇▆▆▁
solid-router.minimal 35.79 KiB 0 B (0.00%) 107.23 KiB 32.15 KiB ███████▆▆▆▁
solid-router.full 40.12 KiB 0 B (0.00%) 120.28 KiB 36.04 KiB ███████▇▆▆▁
vue-router.minimal 51.64 KiB 0 B (0.00%) 147.23 KiB 46.50 KiB █████▇▇▆▆▆▁
vue-router.full 56.48 KiB 0 B (0.00%) 162.82 KiB 50.73 KiB █████▇▇▆▆▆▁
react-start.minimal 99.07 KiB 0 B (0.00%) 311.22 KiB 85.77 KiB ▇▇▇▇▇▇▇▆▆█▁
react-start.full 102.41 KiB 0 B (0.00%) 321.00 KiB 88.55 KiB ███████▆▅▆▁
solid-start.minimal 48.10 KiB 0 B (0.00%) 144.80 KiB 42.54 KiB █████▇▇▆▅█▁
solid-start.full 53.57 KiB 0 B (0.00%) 160.71 KiB 47.17 KiB ███████▆▆█▁

Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 23, 2026

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@6746

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@6746

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@6746

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/nitro-v2-vite-plugin@6746

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@6746

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@6746

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-ssr-query@6746

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@6746

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@6746

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@6746

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@6746

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@6746

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@6746

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@6746

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@6746

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@6746

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@6746

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@6746

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@6746

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@6746

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@6746

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@6746

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@6746

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@6746

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@6746

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@6746

@tanstack/start-fn-stubs

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-fn-stubs@6746

@tanstack/start-plugin-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-plugin-core@6746

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@6746

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-static-server-functions@6746

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@6746

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@6746

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@6746

@tanstack/vue-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router@6746

@tanstack/vue-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router-devtools@6746

@tanstack/vue-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router-ssr-query@6746

@tanstack/vue-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start@6746

@tanstack/vue-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start-client@6746

@tanstack/vue-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start-server@6746

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@6746

commit: 755a654

@schiller-manuel schiller-manuel force-pushed the import-protection-robustness branch from 1904b5c to 7c58156 Compare February 23, 2026 19:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/start-plugin-core/src/import-protection-plugin/plugin.ts (1)

561-587: ⚠️ Potential issue | 🟠 Major

Clear deferred/pending violation state (and reset the counter) at build start.

deferredBuildViolations (and pendingViolations) are never cleared, so watch builds can accumulate stale entries and grow unbounded. Reset them in buildStart, and reset the build mock counter so IDs don’t grow forever.

🛠️ Suggested fix (in buildStart)
       for (const envState of envStates.values()) {
         envState.resolveCache.clear()
         envState.resolveCacheByFile.clear()
         envState.importLocCache.clear()
         envState.seenViolations.clear()
         envState.transformResultCache.clear()
         envState.transformResultKeysByFile.clear()
         envState.postTransformImports.clear()
         envState.serverFnLookupModules.clear()
         envState.graph.clear()
         envState.deniedSources.clear()
         envState.deniedEdges.clear()
         envState.mockExportsByImporter.clear()
+        envState.pendingViolations.clear()
+        envState.deferredBuildViolations.length = 0
       }
+
+      buildViolationCounter = 0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts` around
lines 561 - 587, At build start (inside the buildStart function) clear
accumulated violation state to avoid unbounded growth by resetting
envState.deferredBuildViolations = [] and envState.pendingViolations.clear();
also reset the build mock id counter (buildMockCounter = 0) so generated mock
IDs don’t grow forever; locate envState and the buildStart implementation and
add these resets there (and ensure envState is defined before you clear these
fields).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx`:
- Line 8: The import mixes value and type imports which violates
import/consistent-type-specifier-style and sort-imports; split into two imports
from '../violations/barrel-reexport': keep the value imports "getUsers" and
"userColumns" together (sorted) in one statement and add a separate type-only
import "import type { User } from '../violations/barrel-reexport'"; ensure the
value import appears before the type-only import to satisfy ordering.

In
`@e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts`:
- Around line 7-10: The current side-effect log prints the full DATABASE_URL
(variable DATABASE_URL) which may contain credentials; change the console.log
call in this module to avoid exposing secrets by either logging a redacted
connection string (mask username/password from DATABASE_URL before logging) or
simply logging a static/redacted message (e.g., "[db] connecting (redacted)") so
the module load remains detectable without emitting sensitive data; update the
console.log usage accordingly and keep the DATABASE_URL value assignment
unchanged.

In `@packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md`:
- Around line 53-60: The fenced code blocks in INTERNALS.md are missing language
identifiers (MD040); update the three examples so the first code fence around
the db/index.ts / route.tsx snippet uses a language like "text" and the second
fence around the shouldDefer example uses "ts". Locate the blocks containing the
db/index.ts → export { getUsers } / route.tsx → import { getUsers, userColumns }
snippet and the block with "shouldDefer = (isDevMock && !isPreTransformResolve)
|| isBuild" and change their opening backticks to ```text and ```ts respectively
so markdownlint stops flagging missing languages.

In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts`:
- Around line 1450-1494: Deferred build violations skip the configured
onViolation hook; update generateBundle to invoke config.onViolation(info) for
each real violation and only proceed to this.error or this.warn if the hook does
not indicate suppression. Specifically, inside generateBundle where
realViolations is built, iterate realViolations and call
config.onViolation(info) (or await if it's async) and filter out suppressed
entries before: (a) in error mode, calling this.error(formatViolation(...)) on
the first non-suppressed violation; and (b) in mock/warn mode, deduplicating via
dedupeKey and calling this.warn(formatViolation(...)) only for non-suppressed,
unseen violations; keep existing references to env.deferredBuildViolations,
formatViolation, dedupeKey, this.error and this.warn.

---

Outside diff comments:
In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts`:
- Around line 561-587: At build start (inside the buildStart function) clear
accumulated violation state to avoid unbounded growth by resetting
envState.deferredBuildViolations = [] and envState.pendingViolations.clear();
also reset the build mock id counter (buildMockCounter = 0) so generated mock
IDs don’t grow forever; locate envState and the buildStart implementation and
add these resets there (and ensure envState is defined before you clear these
fields).

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c137bbc and 1904b5c.

📒 Files selected for processing (15)
  • docs/start/framework/react/guide/import-protection.md
  • e2e/react-start/import-protection/src/routes/__root.tsx
  • e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx
  • e2e/react-start/import-protection/src/routes/component-server-leak.tsx
  • e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/foo.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/index.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/shared.ts
  • e2e/react-start/import-protection/src/violations/db-credentials.server.ts
  • e2e/react-start/import-protection/tests/error-mode.setup.ts
  • e2e/react-start/import-protection/tests/import-protection.spec.ts
  • e2e/react-start/import-protection/tests/violations.setup.ts
  • packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md
  • packages/start-plugin-core/src/import-protection-plugin/plugin.ts
  • packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts

// `userColumns` (from ./shared) is a plain object used in JSX — not server-only.
// Tree-shaking should eliminate the ./db.server dependency entirely from the
// client bundle, so no import-protection violation should fire for it.
import { getUsers, userColumns, type User } from '../violations/barrel-reexport'
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Split the type-only import and keep value imports sorted.

ESLint flags inline type specifiers and ordering here. Separate User into a type-only import to satisfy import/consistent-type-specifier-style and sort-imports.

🧹 Suggested fix
-import { getUsers, userColumns, type User } from '../violations/barrel-reexport'
+import { getUsers, userColumns } from '../violations/barrel-reexport'
+import type { User } from '../violations/barrel-reexport'
🧰 Tools
🪛 ESLint

[error] 8-8: Member 'User' of the import declaration should be sorted alphabetically.

(sort-imports)


[error] 8-8: Prefer using a top-level type-only import instead of inline type specifiers.

(import/consistent-type-specifier-style)

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

In `@e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx` at
line 8, The import mixes value and type imports which violates
import/consistent-type-specifier-style and sort-imports; split into two imports
from '../violations/barrel-reexport': keep the value imports "getUsers" and
"userColumns" together (sorted) in one statement and add a separate type-only
import "import type { User } from '../violations/barrel-reexport'"; ensure the
value import appears before the type-only import to satisfy ordering.

Comment on lines +7 to +10
const DATABASE_URL =
process.env.DATABASE_URL ?? 'postgres://admin:s3cret@localhost:5432/myapp'

console.log(`[db] connecting to ${DATABASE_URL}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid logging credentials — use a redacted connection string in the side-effect log.

The default fallback on line 8 embeds plaintext credentials (admin:s3cret), and line 10 immediately logs the full DATABASE_URL (credentials and all). While these are fake credentials, this fixture is documentation-level example code. The pattern is problematic in two ways:

  1. If process.env.DATABASE_URL is populated with real credentials in CI, they would be emitted to build/test logs.
  2. It models the anti-pattern of logging raw connection strings to readers who may copy this pattern.

Since the only requirement of the side-effect is that it be detectable (i.e., proves the module was loaded), logging a redacted or static string is sufficient.

🔒 Proposed fix
-console.log(`[db] connecting to ${DATABASE_URL}`)
+// Log a redacted URL so credentials are never emitted even if the env var is real.
+console.log(`[db] connecting to ${DATABASE_URL.replace(/:\/\/[^@]+@/, '://<redacted>@')}`)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const DATABASE_URL =
process.env.DATABASE_URL ?? 'postgres://admin:s3cret@localhost:5432/myapp'
console.log(`[db] connecting to ${DATABASE_URL}`)
const DATABASE_URL =
process.env.DATABASE_URL ?? 'postgres://admin:s3cret@localhost:5432/myapp'
// Log a redacted URL so credentials are never emitted even if the env var is real.
console.log(`[db] connecting to ${DATABASE_URL.replace(/:\/\/[^@]+@/, '://<redacted>@')}`)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts`
around lines 7 - 10, The current side-effect log prints the full DATABASE_URL
(variable DATABASE_URL) which may contain credentials; change the console.log
call in this module to avoid exposing secrets by either logging a redacted
connection string (mask username/password from DATABASE_URL before logging) or
simply logging a static/redacted message (e.g., "[db] connecting (redacted)") so
the module load remains detectable without emitting sensitive data; update the
console.log usage accordingly and keep the DATABASE_URL value assignment
unchanged.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx (1)

1-8: Split type-only import to satisfy lint rules.
Line 8 matches a prior review note about separating type User and sorting members.

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

In `@e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx`
around lines 1 - 8, Split the mixed import so the runtime values and the type
are imported separately: keep the runtime import for getUsers and userColumns as
a plain import (import { getUsers, userColumns } from '.../barrel-reexport') and
add a separate type-only import for User using the TypeScript syntax (import
type { User } from '.../barrel-reexport'); also sort the named imports
alphabetically in each statement if your lint requires deterministic ordering.
packages/start-plugin-core/src/import-protection-plugin/plugin.ts (1)

1471-1489: Honor onViolation in build-mode reporting

Build-mode generateBundle still reports every surviving violation without calling config.onViolation, so suppression/custom handling doesn’t work in build. Please apply the hook before erroring/warning.

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

In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts` around
lines 1471 - 1489, The build-phase reporting currently skips calling
config.onViolation; update the generateBundle logic so that for every surviving
violation in realViolations you invoke config.onViolation(info) (or await it if
it can be async) before deciding to fail or warn: in the 'error' branch call
config.onViolation(realViolations[0]) before this.error(formatViolation(...)),
and in the mock/warn branch call config.onViolation(info) for each deduped info
(before this.warn(formatViolation(...))); use the existing symbols
realViolations, config.effectiveBehavior, config.onViolation, dedupeKey,
formatViolation, this.error and this.warn to locate and modify the code.
packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md (1)

53-60: Add language identifiers to fenced code blocks (MD040)

The two fenced blocks are missing language tags. Use text for the path diagram and ts for the shouldDefer snippet.

Also applies to: 89-91

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

In `@packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md` around
lines 53 - 60, Add language identifiers to the markdown fenced code blocks: for
the path/exports diagram that shows "db/index.ts → export { getUsers } ..." and
"route.tsx → import { getUsers, userColumns } ..." add the text language tag
(```text), and for the code snippet demonstrating shouldDefer (the TypeScript
example later in the file) add the ts language tag (```ts); also update the
other similar fenced block at lines ~89-91 to use the appropriate language tag
so all fenced blocks include language identifiers.
🤖 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/start-plugin-core/src/import-protection-plugin/plugin.ts`:
- Around line 585-586: The deferredBuildViolations array (declared as
deferredBuildViolations) is not cleared during rebuilds, causing stale entries
to persist; update the buildStart handler (the buildStart method/function that
currently clears per-environment caches and pendingViolations) to also reset
deferredBuildViolations = [] whenever you clear per-env caches so
deferredBuildViolations is fresh on each build/watch iteration.

---

Duplicate comments:
In `@e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx`:
- Around line 1-8: Split the mixed import so the runtime values and the type are
imported separately: keep the runtime import for getUsers and userColumns as a
plain import (import { getUsers, userColumns } from '.../barrel-reexport') and
add a separate type-only import for User using the TypeScript syntax (import
type { User } from '.../barrel-reexport'); also sort the named imports
alphabetically in each statement if your lint requires deterministic ordering.

In `@packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md`:
- Around line 53-60: Add language identifiers to the markdown fenced code
blocks: for the path/exports diagram that shows "db/index.ts → export { getUsers
} ..." and "route.tsx → import { getUsers, userColumns } ..." add the text
language tag (```text), and for the code snippet demonstrating shouldDefer (the
TypeScript example later in the file) add the ts language tag (```ts); also
update the other similar fenced block at lines ~89-91 to use the appropriate
language tag so all fenced blocks include language identifiers.

In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts`:
- Around line 1471-1489: The build-phase reporting currently skips calling
config.onViolation; update the generateBundle logic so that for every surviving
violation in realViolations you invoke config.onViolation(info) (or await it if
it can be async) before deciding to fail or warn: in the 'error' branch call
config.onViolation(realViolations[0]) before this.error(formatViolation(...)),
and in the mock/warn branch call config.onViolation(info) for each deduped info
(before this.warn(formatViolation(...))); use the existing symbols
realViolations, config.effectiveBehavior, config.onViolation, dedupeKey,
formatViolation, this.error and this.warn to locate and modify the code.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1904b5c and 7c58156.

📒 Files selected for processing (15)
  • docs/start/framework/react/guide/import-protection.md
  • e2e/react-start/import-protection/src/routes/__root.tsx
  • e2e/react-start/import-protection/src/routes/barrel-false-positive.tsx
  • e2e/react-start/import-protection/src/routes/component-server-leak.tsx
  • e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/foo.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/index.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/shared.ts
  • e2e/react-start/import-protection/src/violations/db-credentials.server.ts
  • e2e/react-start/import-protection/tests/error-mode.setup.ts
  • e2e/react-start/import-protection/tests/import-protection.spec.ts
  • e2e/react-start/import-protection/tests/violations.setup.ts
  • packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md
  • packages/start-plugin-core/src/import-protection-plugin/plugin.ts
  • packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • e2e/react-start/import-protection/tests/error-mode.setup.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/db.server.ts
  • e2e/react-start/import-protection/tests/import-protection.spec.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/foo.ts
  • packages/start-plugin-core/src/import-protection-plugin/virtualModules.ts
  • e2e/react-start/import-protection/src/violations/barrel-reexport/shared.ts
  • docs/start/framework/react/guide/import-protection.md

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
packages/start-plugin-core/src/import-protection-plugin/plugin.ts (1)

1065-1086: deferredBuildViolations and buildViolationCounter not reset between builds.

The buildStart loop clears every per-env cache except deferredBuildViolations (line 1086). In watch/rebuild scenarios, stale entries accumulate. Because buildViolationCounter is also never reset, the old mock IDs (from prior builds) won't appear in the new bundle's surviving modules — so there's no false-positive/negative — but the array grows as a memory leak.

Both should be reset in buildStart:

🧹 Suggested fix
         for (const envState of envStates.values()) {
           envState.resolveCache.clear()
           envState.resolveCacheByFile.clear()
           envState.importLocCache.clear()
           envState.seenViolations.clear()
           envState.transformResultCache.clear()
           envState.transformResultKeysByFile.clear()
           envState.postTransformImports.clear()
           envState.serverFnLookupModules.clear()
           envState.graph.clear()
           envState.deniedSources.clear()
           envState.deniedEdges.clear()
           envState.mockExportsByImporter.clear()
+          envState.deferredBuildViolations.length = 0
         }

And reset the counter alongside (line 1065 scope):

       buildStart() {
         if (!config.enabled) return
+        buildViolationCounter = 0
         // Clear memoization caches ...
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts` around
lines 1065 - 1086, Reset the stale per-build state by clearing
deferredBuildViolations and resetting buildViolationCounter at the start of each
build: inside buildStart (alongside where you clear per-env caches and the
envStates loop), ensure deferredBuildViolations is cleared (or assigned a fresh
empty array) and buildViolationCounter is set back to 0 so previous-build
entries do not accumulate and leak memory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts`:
- Around line 1065-1086: Reset the stale per-build state by clearing
deferredBuildViolations and resetting buildViolationCounter at the start of each
build: inside buildStart (alongside where you clear per-env caches and the
envStates loop), ensure deferredBuildViolations is cleared (or assigned a fresh
empty array) and buildViolationCounter is set back to 0 so previous-build
entries do not accumulate and leak memory.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c58156 and 16db71e.

📒 Files selected for processing (4)
  • docs/start/framework/react/guide/import-protection.md
  • packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md
  • packages/start-plugin-core/src/import-protection-plugin/plugin.ts
  • packages/start-plugin-core/src/schema.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md (1)

1-290: LGTM — well-structured and technically accurate.

The document clearly covers all key design axes, the false-positive problem and its mitigations, per-environment state management, and the build "mock-first, verify-later" pattern. The previously flagged MD040 fenced-code-language issues are resolved.

One minor optional nit on lines 125–126: the LanguageTool style hint flags three consecutive sentences beginning with "If" (If reachable → confirm. If unreachable → discard. If unknown → keep pending.). The parallel structure is deliberate and idiomatic for exhaustive-condition documentation, but it could be reformatted as a nested bullet list for slightly better readability if desired.

🧹 Optional: reformat as a nested list
-  `processPendingViolations()` checks graph reachability from entry points
-  using only post-transform edges. If the violating importer is reachable
-  → confirm (warn). If unreachable → discard. If unknown → keep pending.
+  `processPendingViolations()` checks graph reachability from entry points
+  using only post-transform edges:
+  - Reachable → confirm (warn).
+  - Unreachable → discard.
+  - Unknown → keep pending.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md` around
lines 1 - 290, The three consecutive "If" sentences in the "Dev + Mock" section
("If reachable → confirm. If unreachable → discard. If unknown → keep pending.")
are flagged for stylistic repetition; reformat that fragment into a nested
bullet list under the "Dev + Mock" heading (or under processPendingViolations()
discussion) so each condition is a separate bullet with the outcome, preserving
the exact meaning and brevity.
🤖 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/start-plugin-core/src/import-protection-plugin/INTERNALS.md`:
- Around line 1-290: The three consecutive "If" sentences in the "Dev + Mock"
section ("If reachable → confirm. If unreachable → discard. If unknown → keep
pending.") are flagged for stylistic repetition; reformat that fragment into a
nested bullet list under the "Dev + Mock" heading (or under
processPendingViolations() discussion) so each condition is a separate bullet
with the outcome, preserving the exact meaning and brevity.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16db71e and 755a654.

📒 Files selected for processing (2)
  • packages/start-plugin-core/src/import-protection-plugin/INTERNALS.md
  • packages/start-plugin-core/src/import-protection-plugin/plugin.ts

@schiller-manuel schiller-manuel merged commit 851967c into main Feb 23, 2026
8 checks passed
@schiller-manuel schiller-manuel deleted the import-protection-robustness branch February 23, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Everything documentation related package: start-plugin-core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants