feat: first draft of tanstack-start-solid-v2#817
feat: first draft of tanstack-start-solid-v2#817brenelz wants to merge 25 commits intoTanStack:mainfrom
Conversation
👷 Deploy request for tanstack pending review.Visit the deploys page to approve it
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new blog post announcing Solid 2.0 beta support across TanStack Router, Start, and Query, and updates package resolution fallback logic in PackageManagerTabs so it can render using the first available framework when the requested framework has no package groups (instead of returning null). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fall back to the first available framework so package manager tabs still render when a page only defines one framework.
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 `@src/components/markdown/PackageManagerTabs.tsx`:
- Around line 55-64: The current selection of packageGroups uses
packagesByFramework[normalizedFramework] which may be an empty array and thus
prevents falling back; update the logic that computes packageGroups (referencing
packagesByFramework, normalizedFramework, and fallbackFramework) to prefer a
non-empty array for normalizedFramework and only use fallbackFramework if the
normalizedFramework entry is missing or empty, and then adjust the subsequent
empty-check to return null only when packageGroups is undefined or has zero
length.
🪄 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: e4fc1fc0-d1e6-4289-885d-8db2fdb42ac7
📒 Files selected for processing (1)
src/components/markdown/PackageManagerTabs.tsx
| const fallbackFramework = Object.keys(packagesByFramework).find( | ||
| (framework) => packagesByFramework[framework]?.length, | ||
| ) | ||
| const packageGroups = | ||
| packagesByFramework[normalizedFramework] || | ||
| (fallbackFramework ? packagesByFramework[fallbackFramework] : undefined) | ||
|
|
||
| // Hide component if current framework not in package list | ||
| // Fall back to the first available framework so single-framework content still renders. | ||
| if (!packageGroups || packageGroups.length === 0) { | ||
| return null |
There was a problem hiding this comment.
Fallback is skipped when the requested framework exists but has zero package groups.
At Line 59, packagesByFramework[normalizedFramework] can be [] (truthy), so the fallback is never used; then Line 63 returns null. That contradicts the fallback intent in Line 62.
Proposed fix
const fallbackFramework = Object.keys(packagesByFramework).find(
(framework) => packagesByFramework[framework]?.length,
)
- const packageGroups =
- packagesByFramework[normalizedFramework] ||
- (fallbackFramework ? packagesByFramework[fallbackFramework] : undefined)
+ const requestedPackageGroups = packagesByFramework[normalizedFramework]
+ const packageGroups =
+ requestedPackageGroups && requestedPackageGroups.length > 0
+ ? requestedPackageGroups
+ : fallbackFramework
+ ? packagesByFramework[fallbackFramework]
+ : undefined📝 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.
| const fallbackFramework = Object.keys(packagesByFramework).find( | |
| (framework) => packagesByFramework[framework]?.length, | |
| ) | |
| const packageGroups = | |
| packagesByFramework[normalizedFramework] || | |
| (fallbackFramework ? packagesByFramework[fallbackFramework] : undefined) | |
| // Hide component if current framework not in package list | |
| // Fall back to the first available framework so single-framework content still renders. | |
| if (!packageGroups || packageGroups.length === 0) { | |
| return null | |
| const fallbackFramework = Object.keys(packagesByFramework).find( | |
| (framework) => packagesByFramework[framework]?.length, | |
| ) | |
| const requestedPackageGroups = packagesByFramework[normalizedFramework] | |
| const packageGroups = | |
| requestedPackageGroups && requestedPackageGroups.length > 0 | |
| ? requestedPackageGroups | |
| : fallbackFramework | |
| ? packagesByFramework[fallbackFramework] | |
| : undefined | |
| // Fall back to the first available framework so single-framework content still renders. | |
| if (!packageGroups || packageGroups.length === 0) { | |
| return null |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/markdown/PackageManagerTabs.tsx` around lines 55 - 64, The
current selection of packageGroups uses packagesByFramework[normalizedFramework]
which may be an empty array and thus prevents falling back; update the logic
that computes packageGroups (referencing packagesByFramework,
normalizedFramework, and fallbackFramework) to prefer a non-empty array for
normalizedFramework and only use fallbackFramework if the normalizedFramework
entry is missing or empty, and then adjust the subsequent empty-check to return
null only when packageGroups is undefined or has zero length.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/blog/tanstack-start-solid-v2.md (1)
65-65: Optional tone polish for the feedback CTA.Consider simplifying “that’s exactly why” to “that’s why” for a slightly tighter close.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/blog/tanstack-start-solid-v2.md` at line 65, Edit the sentence containing "that’s exactly why we want feedback now" in the blog copy (the feedback CTA line) and replace "that’s exactly why" with the tighter phrasing "that’s why" so the sentence reads: "This support is still early, and that’s why we want feedback now." Keep punctuation and surrounding wording unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/blog/tanstack-start-solid-v2.md`:
- Line 65: Edit the sentence containing "that’s exactly why we want feedback
now" in the blog copy (the feedback CTA line) and replace "that’s exactly why"
with the tighter phrasing "that’s why" so the sentence reads: "This support is
still early, and that’s why we want feedback now." Keep punctuation and
surrounding wording unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c84c1eb-8af8-4000-85bb-f0b80d2649e3
📒 Files selected for processing (1)
src/blog/tanstack-start-solid-v2.md
Summary by CodeRabbit