-
-
Notifications
You must be signed in to change notification settings - Fork 217
chore(cli): Temporarily pin TanStack versions for Clerk #690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(cli): Temporarily pin TanStack versions for Clerk #690
Conversation
|
@wobsoriano is attempting to deploy a commit to the Better T Stack Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request updates the package.json template for TanStack Start React frontend to conditionally pin Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cli/templates/frontend/react/tanstack-start/package.json.hbs (1)
15-18: Consider tracking mechanism for removing the temporary pin.Since the PR title indicates this is a "temporary" pin, consider how you'll track when to remove it once TanStack/router#5738 is resolved. Options include:
- Creating a follow-up issue to revisit once the upstream bug is fixed
- Adding a comment in the repository's documentation or a separate tracking file
- Monitoring the upstream issue and updating when a fix is released
Note that adding comments directly in this template would cause them to appear in all generated
package.jsonfiles, which is probably not desirable.Also applies to: 33-33
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cli/templates/frontend/react/tanstack-start/package.json.hbs(2 hunks)
🔇 Additional comments (2)
apps/cli/templates/frontend/react/tanstack-start/package.json.hbs (2)
15-18: I need to verify one more detail about @tanstack/react-router-with-query to ensure it's compatible with the newer versions:Verified: version pinning strategy is sound and compatibility is confirmed.
@tanstack/[email protected] is stable, and it only requires @tanstack/react-start as a dependency for SSR contexts, making it compatible with all router versions in use. The version mismatch you questioned—
@tanstack/[email protected]alongside@tanstack/[email protected]—is intentional and compatible: the peerDependencies for @tanstack/[email protected] only require react and react-dom, not a specific @tanstack/react-router version. The 1.134.6 release includes the HMR fix your workaround addresses, while staying below the problematic 1.134.7 version. The overall approach correctly mitigates the server module leakage issue for Clerk auth users.
16-16: Verify intentionality and scope of@tanstackpackage pinning.The bundling issue (TanStack router #5738) is specifically in
@tanstack/[email protected]+, not in@tanstack/react-router-with-query. Since@tanstack/react-router-with-queryis a thin integration layer that depends on@tanstack/react-router(which is presumably being pinned), pinning it separately may be unnecessary. However, without seeing the full conditional pinning logic in your template, it's unclear whether this is intentional or an oversight. Review the diff to confirm which@tanstackpackages are conditionally pinned and verify whetherreact-router-with-queryneeds the same treatment for your specific use case.
|
Thanks for this @wobsoriano ❤️ |
Temporarily pinning TanStack Start versions to fix TanStack/router#5738
Summary by CodeRabbit