stabilize rsc tests#7217
Conversation
📝 WalkthroughWalkthroughA new helper function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 9559a4d
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/rsc/tests/hydration.ts`:
- Line 1: The import currently mixes a value and a type specifier inline; split
them into two top-level imports to satisfy the lint rule: keep the value import
for expect as import { expect } from '@playwright/test' and add a separate
top-level type-only import import type { Page } from '@playwright/test' so the
type Page is imported with the "type" specifier on its own import declaration.
In `@e2e/react-start/rsc/tests/rsc-deferred.spec.ts`:
- Around line 3-4: Add a blank line after the import statement to satisfy the
import/newline-after-import rule: separate the import of waitForHydration from
the constant declaration DEFERRED_DATA_TIMEOUT by inserting an empty line after
the import so the file's top-of-file imports and subsequent declarations are
visually and lint-wise separated.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3facdf98-973b-4992-957b-07d333897fe2
📒 Files selected for processing (30)
e2e/react-start/rsc/tests/hydration.tse2e/react-start/rsc/tests/rsc-async-bundle.spec.tse2e/react-start/rsc/tests/rsc-basic.spec.tse2e/react-start/rsc/tests/rsc-bundle.spec.tse2e/react-start/rsc/tests/rsc-caching.spec.tse2e/react-start/rsc/tests/rsc-client-preload.spec.tse2e/react-start/rsc/tests/rsc-component-slot.spec.tse2e/react-start/rsc/tests/rsc-context.spec.tse2e/react-start/rsc/tests/rsc-css-modules.spec.tse2e/react-start/rsc/tests/rsc-deferred.spec.tse2e/react-start/rsc/tests/rsc-forms.spec.tse2e/react-start/rsc/tests/rsc-global-css.spec.tse2e/react-start/rsc/tests/rsc-hydration.spec.tse2e/react-start/rsc/tests/rsc-link.spec.tse2e/react-start/rsc/tests/rsc-nested-structure.spec.tse2e/react-start/rsc/tests/rsc-nested.spec.tse2e/react-start/rsc/tests/rsc-parallel-flash.spec.tse2e/react-start/rsc/tests/rsc-param-remount.spec.tse2e/react-start/rsc/tests/rsc-react-cache.spec.tse2e/react-start/rsc/tests/rsc-server-not-found.spec.tse2e/react-start/rsc/tests/rsc-server-redirect.spec.tse2e/react-start/rsc/tests/rsc-slot-jsx-args.spec.tse2e/react-start/rsc/tests/rsc-slots.spec.tse2e/react-start/rsc/tests/rsc-ssr-data-only.spec.tse2e/react-start/rsc/tests/rsc-ssr-false.spec.tse2e/react-start/rsc/tests/rsc-stream-flash.spec.tse2e/react-start/rsc/tests/rsc-stream-generator.spec.tse2e/react-start/rsc/tests/rsc-stream-readable.spec.tse2e/react-start/rsc/tests/rsc-suspense.spec.tse2e/react-start/rsc/tests/rsc-tree.spec.ts
| @@ -0,0 +1,7 @@ | |||
| import { expect, type Page } from '@playwright/test' | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify relevant lint rules are configured in repo lint files
rg -n --iglob '*eslint*' 'import/consistent-type-specifier-style|sort-imports'
# Verify current import in hydration helper
rg -n "import \{ expect, type Page \} from '@playwright/test'" e2e/react-start/rsc/tests/hydration.tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if file exists and read the actual import
cat -n e2e/react-start/rsc/tests/hydration.ts | head -20Repository: TanStack/router
Length of output: 315
🏁 Script executed:
# Find ESLint configuration files in the repo
find . -maxdepth 3 -type f \( -name ".eslintrc*" -o -name "eslint.config.*" \) | head -20Repository: TanStack/router
Length of output: 916
🏁 Script executed:
# Search for import-related lint rules more broadly
rg -A 5 -B 5 "import/consistent-type-specifier-style|sort-imports" --type-list | head -50Repository: TanStack/router
Length of output: 448
🏁 Script executed:
# Read root ESLint config
cat ./eslint.config.js | head -100Repository: TanStack/router
Length of output: 818
🏁 Script executed:
# Read e2e ESLint config
cat ./e2e/eslint-plugin-start/eslint.config.jsRepository: TanStack/router
Length of output: 645
🏁 Script executed:
# Check for any other e2e related eslint configs
find ./e2e -type f -name "eslint.config.*"Repository: TanStack/router
Length of output: 342
🏁 Script executed:
# Search for import-related rules in the codebase
rg "import/consistent-type-specifier-style|sort-imports|simple-import-sort" . --type-list 2>/dev/null | head -30Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Try to find where `@tanstack/eslint-config` is defined or check imports in root config
rg "import/consistent-type-specifier-style|type.*import" --type js --type ts . | head -30Repository: TanStack/router
Length of output: 3853
🏁 Script executed:
# Check the specific e2e/react-start/rsc/eslint.config.js config
cat ./e2e/react-start/rsc/eslint.config.jsRepository: TanStack/router
Length of output: 617
🏁 Script executed:
# Search for any ESLint comments or disable directives in the hydration.ts file
cat -n e2e/react-start/rsc/tests/hydration.tsRepository: TanStack/router
Length of output: 315
🏁 Script executed:
# Check if `@tanstack/eslint-config` has type-import rules
rg "import/consistent-type-specifier-style" . --type js --type tsRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check if there's a parent or global eslint config for e2e tests
find ./e2e -name ".eslintignore" -o -name "eslint.config.*" | xargs ls -la 2>/dev/nullRepository: TanStack/router
Length of output: 636
🏁 Script executed:
# Search for any eslint overrides or configs that might target tests or hydration.ts
rg "tests.*hydration|hydration\.ts" . --type js --type ts --type jsonRepository: TanStack/router
Length of output: 553
🏁 Script executed:
# Check if there's any playwright-specific eslint config or rules
rg "playwright|@playwright" . --type js --type ts -A 3 | grep -i "rule\|config" | head -20Repository: TanStack/router
Length of output: 1668
🏁 Script executed:
# Check what rules `@tanstack/eslint-config` exports (look for it in node_modules or package.json)
rg "tanstack.*eslint.*config" . --type json -A 3 | head -30Repository: TanStack/router
Length of output: 905
🏁 Script executed:
# Check if there are any eslint overrides or additional configs in root eslint.config.js that might apply to test files
cat ./eslint.config.js | grep -A 20 -B 5 "test\|e2e"Repository: TanStack/router
Length of output: 41
🏁 Script executed:
# Look for any other ESLint configs in parent directories or check if test files have any lint rules
rg "import.*type|type.*import" e2e/react-start/rsc/tests/ --type ts -B 2 -A 2Repository: TanStack/router
Length of output: 1317
🏁 Script executed:
# Check if other test files in same directory use the same pattern
rg "import.*type.*from.*@playwright/test" e2e/react-start/rsc/tests/ -lRepository: TanStack/router
Length of output: 141
🏁 Script executed:
# Verify the root eslint.config.js doesn't have any overrides for test files
cat ./eslint.config.jsRepository: TanStack/router
Length of output: 818
🏁 Script executed:
# Check if `@tanstack/eslint-config` actually includes import/consistent-type-specifier-style
npm list `@tanstack/eslint-config` 2>/dev/null || grep "@tanstack/eslint-config" ./package.jsonRepository: TanStack/router
Length of output: 738
🏁 Script executed:
# Find and examine the `@tanstack/eslint-config` to see what rules it exports
find ./node_modules -path "*@tanstack/eslint-config*" -name "*.js" -o -path "*@tanstack/eslint-config*" -name "*.ts" | head -10Repository: TanStack/router
Length of output: 2001
🏁 Script executed:
# Check the actual eslint-config content
cat ./node_modules/.pnpm/@tanstack+eslint-config@0.4.0_@typescript-eslint+utils@8.57.1_eslint@9.22.0_jiti@2.6.1__71a7465028deca506823eb713e9f533e/node_modules/@tanstack/eslint-config/dist/index.js 2>/dev/null | head -200Repository: TanStack/router
Length of output: 1802
🏁 Script executed:
# Search for import/consistent-type-specifier-style in entire node_modules to see if it's even available
rg "import/consistent-type-specifier-style" ./node_modules --max-count 5 2>/dev/nullRepository: TanStack/router
Length of output: 41
🏁 Script executed:
# Check what import rules are defined in `@tanstack/eslint-config`
cat ./node_modules/.pnpm/@tanstack+eslint-config@0.4.0_@typescript-eslint+utils@8.57.1_eslint@9.22.0_jiti@2.6.1__71a7465028deca506823eb713e9f533e/node_modules/@tanstack/eslint-config/dist/import.jsRepository: TanStack/router
Length of output: 592
🏁 Script executed:
# Check if eslint-plugin-import-x (which is used) has the consistent-type-specifier-style rule
rg "consistent-type-specifier-style" ./node_modules/.pnpm/eslint-plugin-import-x* --max-count 5 2>/dev/null | head -10Repository: TanStack/router
Length of output: 41
Separate the type import to top level to satisfy lint rules.
The file uses inline type Page specifier which violates the configured import/consistent-type-specifier-style rule (set to "error" with "prefer-top-level" style in @tanstack/eslint-config).
♻️ Proposed fix
-import { expect, type Page } from '@playwright/test'
+import { expect } from '@playwright/test'
+import type { Page } 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 { expect, type Page } from '@playwright/test' | |
| import { expect } from '@playwright/test' | |
| import type { Page } from '@playwright/test' |
🧰 Tools
🪛 ESLint
[error] 1-1: Member 'Page' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 1-1: 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/rsc/tests/hydration.ts` at line 1, The import currently mixes
a value and a type specifier inline; split them into two top-level imports to
satisfy the lint rule: keep the value import for expect as import { expect }
from '@playwright/test' and add a separate top-level type-only import import
type { Page } from '@playwright/test' so the type Page is imported with the
"type" specifier on its own import declaration.
| import { waitForHydration } from './hydration' | ||
| const DEFERRED_DATA_TIMEOUT = 5000 // Deferred data takes ~2 seconds |
There was a problem hiding this comment.
Fix lint error: add a blank line after imports.
Line 3 is currently followed by a declaration on Line 4, which violates import/newline-after-import reported by ESLint.
Suggested fix
import { expect } from '@playwright/test'
import { test } from '@tanstack/router-e2e-utils'
import { waitForHydration } from './hydration'
+
const DEFERRED_DATA_TIMEOUT = 5000 // Deferred data takes ~2 seconds📝 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 { waitForHydration } from './hydration' | |
| const DEFERRED_DATA_TIMEOUT = 5000 // Deferred data takes ~2 seconds | |
| import { expect } from '@playwright/test' | |
| import { test } from '@tanstack/router-e2e-utils' | |
| import { waitForHydration } from './hydration' | |
| const DEFERRED_DATA_TIMEOUT = 5000 // Deferred data takes ~2 seconds |
🧰 Tools
🪛 ESLint
[error] 3-3: Expected 1 empty line after import statement not followed by another import.
(import/newline-after-import)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/react-start/rsc/tests/rsc-deferred.spec.ts` around lines 3 - 4, Add a
blank line after the import statement to satisfy the import/newline-after-import
rule: separate the import of waitForHydration from the constant declaration
DEFERRED_DATA_TIMEOUT by inserting an empty line after the import so the file's
top-of-file imports and subsequent declarations are visually and lint-wise
separated.
1 participant
Summary by CodeRabbit