-
-
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: inert value in next15 #4491
base: canary
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 2d1d692 The changes in this PR will be included in the next version bump. This PR includes changesets to release 58 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 ↗︎
|
WalkthroughThis pull request introduces changes to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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
|
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: 1
🧹 Nitpick comments (3)
packages/utilities/shared-utils/src/functions.ts (2)
404-413
: Consider using semver for robust version checks.
Dynamically importingnext/package.json
might introduce overhead and partial reliability issues if the version format changes. Relying onstartsWith("15")
in other parts of the code is also somewhat fragile. Using a semver library would provide a more resilient approach.
415-419
: Handle minor & patch releases when identifying Next.js 15.
Usingversion.startsWith("15")
might work for now, but it may fail to capture or properly handle any future minor/patch releases (e.g., 15.10.x). A semantic version check would be more maintainable long-term..changeset/smart-eagles-hope.md (1)
1-5
: Enhance the patch description for clarity.
Consider adding more details about this fix, including how it impacts Tabs or addresses the Next.js 15 error, to help future maintainers and users understand the scope of the patch more readily.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/smart-eagles-hope.md
(1 hunks)packages/components/tabs/src/tab-panel.tsx
(0 hunks)packages/utilities/shared-utils/src/functions.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/components/tabs/src/tab-panel.tsx
e5415f0
to
7b779e4
Compare
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: 4
🧹 Nitpick comments (3)
packages/utilities/shared-utils/README.md (2)
21-29
: Add language specification to code blockThe file structure code block should specify a language for better syntax highlighting.
-``` +```text src/ ├── common/ # Common utilities for all React versions └── demi/ # Demi utilities for different React versions ├── react18/ └── react19/🧰 Tools
🪛 Markdownlint (0.37.0)
23-23: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Line range hint
1-34
: Enhance documentation with version compatibility detailsConsider adding a section about React version compatibility and how the package handles different versions.
Add the following section:
+## Version Compatibility + +This package automatically adapts to your React version: +- React 18 / Next.js 14: Uses react18 utilities +- React 19 / Next.js 15+: Uses react19 utilities + ## License🧰 Tools
🪛 Markdownlint (0.37.0)
23-23: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
packages/utilities/shared-utils/src/demi/react18/getInertValue.ts (1)
1-17
: Consider adding type guards for better type safetyWhile the implementation is correct, you might want to consider adding type guards to ensure type safety when consuming the function.
-export const getInertValue = (v: boolean): boolean | string | undefined => { +export const getInertValue = (v: boolean): "" | undefined => { return v ? "" : undefined; };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.changeset/smart-eagles-hope.md
(1 hunks)packages/utilities/shared-utils/README.md
(3 hunks)packages/utilities/shared-utils/package.json
(3 hunks)packages/utilities/shared-utils/scripts/postinstall.js
(1 hunks)packages/utilities/shared-utils/src/common/functions.ts
(0 hunks)packages/utilities/shared-utils/src/common/index.ts
(1 hunks)packages/utilities/shared-utils/src/demi/react18/getInertValue.ts
(1 hunks)packages/utilities/shared-utils/src/demi/react18/index.ts
(1 hunks)packages/utilities/shared-utils/src/demi/react19/getInertValue.ts
(1 hunks)packages/utilities/shared-utils/src/demi/react19/index.ts
(1 hunks)packages/utilities/shared-utils/src/index.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/utilities/shared-utils/src/common/functions.ts
✅ Files skipped from review due to trivial changes (3)
- packages/utilities/shared-utils/src/demi/react18/index.ts
- packages/utilities/shared-utils/src/demi/react19/index.ts
- packages/utilities/shared-utils/src/common/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/smart-eagles-hope.md
🧰 Additional context used
🪛 Markdownlint (0.37.0)
packages/utilities/shared-utils/README.md
23-23: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: TypeScript
- GitHub Check: Build
🔇 Additional comments (7)
packages/utilities/shared-utils/scripts/postinstall.js (1)
4-10
: Well-implemented error handling!The function safely handles package requirements with proper error fallback.
packages/utilities/shared-utils/package.json (1)
49-51
: LGTM: Build configuration changesThe tsup configuration changes appropriately handle the new directory structure with React version-specific entries.
packages/utilities/shared-utils/src/index.ts (2)
1-3
: Clarify the "For Development" commentThe comment "For Development" is ambiguous. Please clarify if this is a temporary state or if there are specific development-related implications.
4-5
: Verify the impact of removed exportsThe removal of multiple utility exports (assertion, clsx, object, text, etc.) could be a breaking change for consumers. Please ensure this is intentional and documented.
✅ Verification successful
No breaking changes detected - utilities are still available
The utilities (assertion, clsx, object, text, etc.) have not been removed but rather reorganized into individual files under the
common/
directory. They remain accessible through the same import path@nextui-org/shared-utils
as evidenced by their continued successful usage throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of removed utilities in the codebase rg -l "import.*from.*@nextui-org/shared-utils.*" | xargs rg "(assertion|clsx|object|text|dimensions|functions|numbers|console|types|dates|regex)"Length of output: 66732
Script:
#!/bin/bash # Check the contents of the remaining export files echo "Contents of ./common:" cat packages/utilities/shared-utils/src/common.ts || echo "File not found" echo -e "\nContents of ./demi/react18:" cat packages/utilities/shared-utils/src/demi/react18.ts || echo "File not found" # Also check if there are other potential utility files echo -e "\nListing all .ts files in shared-utils src:" find packages/utilities/shared-utils/src -name "*.ts" -type fLength of output: 1571
packages/utilities/shared-utils/src/demi/react19/getInertValue.ts (2)
1-14
: Excellent documentation!The JSDoc comment is thorough and well-written, including a clear explanation of the behavior differences between React versions and a reference link.
15-17
: Implementation matches React 19 requirementsThe function correctly returns the boolean value directly for React 19, as specified in the documentation.
packages/utilities/shared-utils/src/demi/react18/getInertValue.ts (1)
15-17
: Implementation matches legacy React requirementsThe function correctly implements the legacy behavior for React 18 and earlier versions, returning an empty string for
true
andundefined
forfalse
.Consider adding unit tests to verify the behavior:
describe('getInertValue', () => { test('should return empty string when true', () => { expect(getInertValue(true)).toBe(''); }); test('should return undefined when false', () => { expect(getInertValue(false)).toBe(undefined); }); });✅ Verification successful
Implementation verified, test coverage recommended
The implementation is correctly handling the legacy React 18 behavior, and its usage across calendar and tabs components is consistent. The original test suggestion remains valid and would help ensure the behavior is maintained.
- Used in
calendar-month.tsx
,calendar-picker.tsx
, andtab-panel.tsx
for managing inert states- Implementation aligns with React version-specific requirements as documented
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for test files related to getInertValue fd -e test.ts -e spec.ts | grep -i inert # Find React 19 implementation fd getInertValue.ts | grep react19 # Search for usages of getInertValue rg "getInertValue" -lLength of output: 563
Script:
#!/bin/bash # Check React 19 implementation cat packages/utilities/shared-utils/src/demi/react19/getInertValue.ts # Check usage in components rg "getInertValue" -A 2 -B 2 packages/components/tabs/src/tab-panel.tsx packages/components/calendar/src/calendar-picker.tsx packages/components/calendar/src/calendar-month.tsxLength of output: 3903
Closes #
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
Release Notes for @nextui-org/shared-utils
Package Updates
@nextui-org/system
to@nextui-org/shared-utils
New Features
getInertValue
function forinert
attribute handlingImprovements
Bug Fixes
Tabs
component in Next.js 15 environmentRefactor