Uh oh!
There was an error while loading. Please reload this page.
fix: avoid false positives in import-protection plugin - #6708
Conversation
📝 WalkthroughWalkthroughThe PR introduces boundary-safe and factory-safe server-function patterns in e2e tests, adds a silent mode to the import-protection plugin for pre-transform resolutions to reduce false positives, exports the SERVER_FN_LOOKUP constant, and updates test infrastructure to use real browser navigation for SSR transforms. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... | ❌ Failed | 14m 43s | View ↗ |
nx run-many --target=build --exclude=examples/*... | ✅ Succeeded | 22s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-02-19 23:55:05 UTC
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/tests/violations.setup.ts`:
- Line 4: The combined import "import { chromium, type FullConfig } from
'@playwright/test'" violates lint rules; separate the type-only specifier into
its own import. Replace the single import with two imports: one importing the
runtime symbol chromium and another importing the type FullConfig using "import
type { FullConfig } from '@playwright/test'"; update any usages of chromium and
FullConfig accordingly (symbols: chromium, FullConfig).
In `@packages/start-plugin-core/src/import-protection-plugin/plugin.ts`:
- Around line 749-767: The code reads (_options as Record<string, unknown>).scan
without guarding that _options may be undefined in the resolveId hook; change
the isPreTransformResolve check to safely access scan (e.g. use optional
chaining or a null-check) so that the expression becomes importer.includes('?' +
SERVER_FN_LOOKUP) || !!((_options as Record<string, unknown>)?.scan), keeping
the rest of the logic in isPreTransformResolve intact and referencing the
existing symbols (_options, isPreTransformResolve, SERVER_FN_LOOKUP).
| import path from 'node:path' | ||
| import { spawn } from 'node:child_process' | ||
| import type { FullConfig } from '@playwright/test' | ||
| import { chromium, type FullConfig } from '@playwright/test' |
There was a problem hiding this comment.
Fix Playwright import style to satisfy lint rules.
The linter flags inline type specifiers and member ordering. Split the type-only import into its own line.
✅ Suggested fix
-import { chromium, type FullConfig } from '@playwright/test'+import { chromium } from '@playwright/test'+import type { FullConfig } from '@playwright/test'📝 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.
| import{chromium,typeFullConfig}from'@playwright/test' | |
| import{chromium}from'@playwright/test' | |
| importtype{FullConfig}from'@playwright/test' |
🧰 Tools
🪛 ESLint
[error] 4-4: Member 'FullConfig' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 4-4: 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/tests/violations.setup.ts` at line 4, The
combined import "import { chromium, type FullConfig } from '@playwright/test'"
violates lint rules; separate the type-only specifier into its own import.
Replace the single import with two imports: one importing the runtime symbol
chromium and another importing the type FullConfig using "import type {
FullConfig } from '@playwright/test'"; update any usages of chromium and
FullConfig accordingly (symbols: chromium, FullConfig).
Uh oh!
There was an error while loading. Please reload this page.
Summary by CodeRabbit
Tests
Bug Fixes
Refactor