Skip to content

Comments

fix: avoid false positives in import protection#6732

Merged
schiller-manuel merged 1 commit intomainfrom
fix-false-positives
Feb 22, 2026
Merged

fix: avoid false positives in import protection#6732
schiller-manuel merged 1 commit intomainfrom
fix-false-positives

Conversation

@schiller-manuel
Copy link
Contributor

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

Summary by CodeRabbit

  • Bug Fixes
    • Reduce false-positive import-protection warnings for cross-boundary server imports in dev and dev.warm; reachability checks are more accurate and flush sooner when entries appear.
  • Tests
    • Added regression coverage for dev and dev.warm modes to prevent variant pollution and validate warm-run behavior.
  • Chores
    • Internal optimizations to import-protection processing for faster, more reliable detection.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 22, 2026

📝 Walkthrough

Walkthrough

Removed per-environment hasSeenEntry flag and now infer entry presence from env.graph.entries.size. Pending violations are deferred as "unknown" until entries exist; SERVER_FN_LOOKUP variants are excluded from reachability checks; pending violations are flushed earlier when entries are discovered via resolveId.

Changes

Cohort / File(s) Summary
Import protection plugin
packages/start-plugin-core/src/import-protection-plugin/plugin.ts
Removed EnvState.hasSeenEntry; replaced uses with env.graph.entries.size > 0. Treat pending violations as 'unknown' when no entries exist. Skip SERVER_FN_LOOKUP_QUERY variants during reachability checks. Flush pending violations immediately when an addon entry is resolved (no importer). Removed initialization/reset/logging for hasSeenEntry.
E2E tests — dev.warm coverage & regression
e2e/react-start/import-protection/tests/import-protection.spec.ts
Added dev.warm mode to test suites, added regression test preventing SERVER_FN_LOOKUP pollution in dev/dev.warm, and fixed a warm-trace boolean edge-case.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • tannerlinsley

Poem

"I'm a rabbit in code, nibbling through the haze,
Flushing old suspicions in bright, careful ways.
I skip the lookup traps and wait for entries' light,
So violations don't shout in the middle of night.
Hop, hop — cleaner imports, and everything's right!" 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main purpose of the changeset: fixing false positives in import protection by removing the hasSeenEntry field and improving violation handling.

✏️ 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 fix-false-positives

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

@nx-cloud
Copy link

nx-cloud bot commented Feb 22, 2026

View your CI Pipeline Execution ↗ for commit 229d8d0

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

☁️ Nx Cloud last updated this comment at 2026-02-22 21:02:50 UTC

@github-actions
Copy link

github-actions bot commented Feb 22, 2026

Bundle Size Benchmarks

  • Commit: 8e9e0c0f3898
  • Measured at: 2026-02-22T20:54:11.110Z
  • Baseline source: history:63e9e71991e9
  • Dashboard: bundle-size history
Scenario Current (gzip) Delta vs baseline Raw Brotli Trend
react-router.minimal 86.78 KiB 0 B (0.00%) 272.59 KiB 75.35 KiB ▁▁▁▁▁█
react-router.full 89.84 KiB 0 B (0.00%) 283.07 KiB 78.09 KiB █████▁
solid-router.minimal 36.03 KiB 0 B (0.00%) 107.76 KiB 32.40 KiB █████▁
solid-router.full 40.38 KiB 0 B (0.00%) 120.84 KiB 36.24 KiB █████▁
vue-router.minimal 51.87 KiB 0 B (0.00%) 147.75 KiB 46.62 KiB █████▁
vue-router.full 56.73 KiB 0 B (0.00%) 163.36 KiB 51.00 KiB █████▁
react-start.minimal 99.25 KiB 0 B (0.00%) 311.59 KiB 85.95 KiB ▁▁▁▁▁█
react-start.full 102.65 KiB 0 B (0.00%) 321.50 KiB 88.74 KiB █████▁
solid-start.minimal 48.27 KiB 0 B (0.00%) 145.19 KiB 42.69 KiB █████▁
solid-start.full 53.75 KiB 0 B (0.00%) 161.14 KiB 47.39 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 22, 2026

More templates

@tanstack/arktype-adapter

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-fn-stubs

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/vue-router

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

@tanstack/vue-router-devtools

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

@tanstack/vue-router-ssr-query

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

@tanstack/vue-start

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

@tanstack/vue-start-client

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

@tanstack/vue-start-server

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

@tanstack/zod-adapter

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

commit: 229d8d0

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)
e2e/react-start/import-protection/tests/import-protection.spec.ts (1)

491-492: Correct fix: prev is always defined in this loop context.

Since the loop starts at i = 1 and the find predicate requires trace.length >= 3, prev = v!.trace[i - 1] is guaranteed to reference a valid element. Removing the unnecessary optional chaining on prev is appropriate.

Consider applying the same fix to line 138 for consistency, where the identical loop structure also guarantees prev is defined.

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

In `@e2e/react-start/import-protection/tests/import-protection.spec.ts` around
lines 491 - 492, The variable prev is provably always defined in the loop (loop
starts at i = 1 and the surrounding find ensures trace.length >= 3), so remove
the unnecessary optional chaining on prev (change
prev.specifier?.includes('?tsr-split=') to
prev.specifier.includes('?tsr-split=')). Also make the identical change at the
other occurrence in the file (the similar loop around the earlier occurrence
near the other trace handling, referenced in the review) so both spots
consistently drop the optional chaining.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@e2e/react-start/import-protection/tests/import-protection.spec.ts`:
- Around line 491-492: The variable prev is provably always defined in the loop
(loop starts at i = 1 and the surrounding find ensures trace.length >= 3), so
remove the unnecessary optional chaining on prev (change
prev.specifier?.includes('?tsr-split=') to
prev.specifier.includes('?tsr-split=')). Also make the identical change at the
other occurrence in the file (the similar loop around the earlier occurrence
near the other trace handling, referenced in the review) so both spots
consistently drop the optional chaining.

@schiller-manuel schiller-manuel merged commit 53eb52a into main Feb 22, 2026
8 checks passed
@schiller-manuel schiller-manuel deleted the fix-false-positives branch February 22, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant