fix: pnpm-lock & typing issues#2601
Conversation
🦋 Changeset detectedLatest commit: f72b3a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 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 ↗︎
|
|
@wingkwong is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe recent updates focus on enhancing type safety and user experience across various components in a UI library. Key changes include casting Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (12)
- .changeset/wise-kangaroos-wonder.md (1 hunks)
- apps/docs/content/components/input/custom-impl.ts (1 hunks)
- packages/components/checkbox/src/checkbox-group.tsx (1 hunks)
- packages/components/input/src/input.tsx (1 hunks)
- packages/components/input/src/textarea.tsx (1 hunks)
- packages/components/input/src/use-input.ts (2 hunks)
- packages/components/input/stories/input.stories.tsx (1 hunks)
- packages/components/navbar/src/use-navbar.ts (1 hunks)
- packages/components/radio/src/radio-group.tsx (1 hunks)
- packages/components/select/src/select.tsx (1 hunks)
- packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts (1 hunks)
- packages/hooks/use-aria-press/src/index.ts (5 hunks)
Files skipped from review due to trivial changes (2)
- .changeset/wise-kangaroos-wonder.md
- packages/components/input/src/textarea.tsx
Additional comments (13)
packages/components/checkbox/src/checkbox-group.tsx (1)
- 29-29: The change to cast
errorMessageasReact.ReactNodealigns with the PR's objective to fix typing issues and ensures type correctness when rendering error messages. Looks good!packages/components/radio/src/radio-group.tsx (1)
- 30-30: Casting
errorMessageasReact.ReactNodehere is consistent with the PR's goal to enhance type safety. Well done!packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts (1)
- 26-26: Adding a generic type
Tto theValidationproperty in theMultiSelectPropsinterface enhances type safety and flexibility. This is a positive change.packages/components/input/src/input.tsx (1)
- 50-50: Casting
errorMessageasReact.ReactNodein this context ensures type correctness and aligns with the PR's objectives. Good job!apps/docs/content/components/input/custom-impl.ts (1)
- 148-148: Casting
errorMessageasReact.ReactNodein the custom implementation of theMyInputcomponent is consistent with the PR's goal to fix typing issues. Nicely done!packages/components/select/src/select.tsx (1)
- 60-60: Casting
errorMessageasReact.ReactNodein theSelectcomponent ensures type correctness and aligns with the PR's objectives. Looks good!packages/components/navbar/src/use-navbar.ts (1)
- 126-126: Explicitly setting the default value for
isMenuOpentofalseusing the nullish coalescing operator improves clarity and predictability. This is a good practice.packages/components/input/src/use-input.ts (2)
- 118-118: Setting a default value for
props.defaultValueto an empty string if it isundefinedis a good practice for ensuring predictable behavior of input components. This enhances the component's robustness.- 155-155: Adding
autoCapitalize: undefinedto the input element configuration is a minor change. While it's not immediately clear why this specific value is chosen, it doesn't seem to introduce any issues. It might be beneficial to clarify the reasoning behind this choice for future reference.packages/components/input/stories/input.stories.tsx (1)
- 453-453: Casting
errorMessageasReact.ReactNodeenhances flexibility in displaying error messages, allowing for a broader range of React nodes. This change looks good.Ensure that all existing usages of
errorMessageare reviewed to confirm compatibility with this broader type.packages/hooks/use-aria-press/src/index.ts (3)
- 85-112: The modifications to
triggerPressStartimprove the robustness of press event handling by preventing redundant or invalid event processing. These changes are well-structured and enhance the logic's clarity and effectiveness.Consider adding comments explaining the early return conditions for
isDisabled,didFirePressStart, andpointerTypebeing null, to aid future maintainers.
- 152-178: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [116-175]
The updates to
triggerPressEndensure that press end events are handled correctly, only after a press start event has been fired and with a valid pointer type. The careful handling ofonPressEnd,onPressChange, andonPressevents, including considerations forisDisabled, enhances the component's accessibility and user experience.Ensure comprehensive testing around these updated logic paths, particularly in scenarios where press events might be interrupted or canceled, to maintain robustness.
- 155-175: The enhancements to
triggerPressUpalign with the overall improvements in press event handling, ensuring consistent and predictable behavior. The careful consideration ofisDisabledandpointerTypeenhances the component's robustness.Review event handling across different browsers and devices to ensure consistent behavior, especially given the nuances of pointer and touch events.
|
Due to our use of the caret ( Therefore, it would be helpful if we could close this PR. Thank you! |
|
As discussed with @ryo-manba, PR2561 would address the issues. It's better to have those changes from his PR due to some dependencies with latest react-aria changes. Therefore, I'm closing this PR. |
3 participants
Closes #
📝 Description
Current Problem:
if we remove pnpm-lock.yaml and run pnpm install again, we can see the changes introduced.
previously we couldn't reproduce some issues reported by users locally but in the build package (e.g. isRequired prop not working in radio group). Tests didn't catch the failed case as well. With the updated pnpm-lock.yaml, the issue can be reproducible & the test fails expectedly.
After updating pnpm-lock.yaml, some typings would fail causing the build to fail. This ticket is to resolve all the build issues. Failing test cases will be handled separately.
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
There are two test cases failing