-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix(calendar): invalid prop ref supplied to React.Fragment #4428
Conversation
🦋 Changeset detectedLatest commit: 0e9cfd3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request addresses a bug in the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (1)packages/components/calendar/src/calendar-base.tsx (1)
The removal of the unnecessary Fragment wrapper around MotionConfig is the correct approach, as Fragments are only needed when grouping multiple children. This change resolves the invalid prop reference issues with React.Fragment while maintaining the component's functionality. Let's verify the React 19 compatibility: ✅ Verification successfulLGTM! The Fragment removal is safe and improves React 19 compatibility. The codebase scan confirms that the Fragment removal in calendar-base.tsx is appropriate. The remaining Fragment usages in the codebase are either:
The change aligns with React best practices by removing unnecessary Fragment wrapper when there's only one child element (MotionConfig). 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any remaining Fragment usage that might cause issues
# with prop references in React 19
# Search for Fragment usage in calendar components
rg -A 2 "Fragment|<>|</>" --type typescript --type typescriptreact
# Look for potential prop passing to Fragments
ast-grep --pattern 'Fragment $props {$$$}'
Length of output: 148 Script: #!/bin/bash
# Search for Fragment usage in calendar components with correct file extensions
rg -A 2 "Fragment|<>|</>" -g "*.tsx" -g "*.ts"
# Look for potential prop passing to Fragments in tsx files
rg -A 2 "Fragment.*props" -g "*.tsx" -g "*.ts"
# Check for any remaining Fragment imports
rg "import.*Fragment" -g "*.tsx" -g "*.ts"
Length of output: 29376 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
ref
supplied toReact.Fragment
. React.Fragment can only havekey
andchildren
props. #4358ref
supplied toReact.Fragment
. React.Fragment can only havekey
andchildren
props. #4068📝 Description
In 2.3.0,
PopLayoutWrapper
was used to forward the ref.At some point (couldn't trace when probably due to force push), it was changed to use React.Fragment, i.e.
<></>
. This is where the error is thrown.In Ripple component, there is the same pattern (i.e. replace
PopLayoutWrapper
by<></>
) and the fragment got removed in this commit. Hence, applying the same change tocalendar-base.tsx
.⛳️ Current behavior (updates)
Sandbox provided by the user: https://codesandbox.io/p/devbox/friendly-diffie-55mxp5
🚀 New behavior
I cloned the above sandbox and link to my local package for testing.
pr4428-demo.webm
💣 Is this a breaking change (Yes/No):
📝 Additional Information