Conversation
📝 WalkthroughWalkthroughThis PR fixes CSS asset ordering in TanStack Start's manifest plugin. The change reorders CSS asset collection so that styles from imported chunks are emitted before the route chunk's own CSS, with corresponding test expectations updated to reflect this new ordering behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 7093fee
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 4 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests/start-manifest-plugin/manifestBuilder.test.ts`:
- Around line 555-557: The test is using an untyped asset parameter in the map
call; replace the cast "(asset: any)" with "(asset: RouterManagedTag)" and add
an import for RouterManagedTag from "@tanstack/router-core" at the top of the
test file so the expression
manifest.routes['/field-detail-panel']!.assets!.map((asset: RouterManagedTag) =>
asset.attrs.href) is strictly typed; ensure the import name matches the symbol
RouterManagedTag used in the map callback.
🪄 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: b1a39da5-4f44-457e-9343-9e801fc2ea49
📒 Files selected for processing (3)
.changeset/start-asset-sorting.mdpackages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.tspackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts
| manifest.routes['/field-detail-panel']!.assets!.map( | ||
| (asset: any) => asset.attrs.href, | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify explicit `any` usage in this test file and inspect context.
rg -nP --type=ts -C2 '\bany\b' packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.tsRepository: TanStack/router
Length of output: 3628
🏁 Script executed:
# Check for type definitions related to assets and manifest
fd -e ts -e tsx packages/start-plugin-core | grep -E '(type|interface|manifest)' | head -20Repository: TanStack/router
Length of output: 469
🏁 Script executed:
# Search for RouterManagedTag definition
rg -n 'RouterManagedTag' packages/start-plugin-core --type=tsRepository: TanStack/router
Length of output: 2792
🏁 Script executed:
# Find the manifest type definition to understand asset typing
rg -n -A5 'assets.*:' packages/start-plugin-core --type=ts | head -40Repository: TanStack/router
Length of output: 4213
🏁 Script executed:
# Check what types are available for the asset mapping pattern
rg -n '\btype.*Asset' packages/start-plugin-core --type=tsRepository: TanStack/router
Length of output: 426
Replace any with RouterManagedTag to maintain strict type safety.
Line 556 uses (asset: any) when mapping assets; use RouterManagedTag (imported from @tanstack/router-core) instead, consistent with the manifest builder's type definitions and TypeScript strict mode requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts`
around lines 555 - 557, The test is using an untyped asset parameter in the map
call; replace the cast "(asset: any)" with "(asset: RouterManagedTag)" and add
an import for RouterManagedTag from "@tanstack/router-core" at the top of the
test file so the expression
manifest.routes['/field-detail-panel']!.assets!.map((asset: RouterManagedTag) =>
asset.attrs.href) is strictly typed; ensure the import name matches the symbol
RouterManagedTag used in the map callback.
Merging this PR will not alter performance
Comparing Footnotes
|
Summary by CodeRabbit