fix: onSelectionChange type incorrect #3336
Conversation
🦋 Changeset detectedLatest commit: 92f8703 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 |
WalkthroughThis change primarily updates the typing for selection-related properties in Changes
Sequence Diagram(s)(No significant new feature or control flow changes warranting a sequence diagram.) Assessment against linked issues
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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/core/system-rsc/src/types.ts (1)
Line range hint
32-33: Replace generic object types with explicit definitions.The use of
{}as a type is discouraged as it allows any non-nullish value. Prefer defining explicit shapes for better type safety.- type RightJoinProps<SourceProps extends object = {}, OverrideProps extends object = {}> = ... + type RightJoinProps<SourceProps extends Record<string, any> = {}, OverrideProps extends Record<string, any> = {}> = ...Also applies to: 33-34, 39-40, 47-48
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- .changeset/chilled-worms-develop.md (1 hunks)
- apps/docs/content/docs/components/dropdown.mdx (2 hunks)
- apps/docs/content/docs/components/select.mdx (2 hunks)
- packages/components/menu/src/use-menu.ts (2 hunks)
- packages/components/select/src/use-select.ts (3 hunks)
- packages/core/system-rsc/src/index.ts (1 hunks)
- packages/core/system-rsc/src/types.ts (2 hunks)
- packages/core/system/src/index.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/core/system-rsc/src/index.ts
- packages/core/system/src/index.ts
Additional context used
Biome
packages/core/system-rsc/src/types.ts
[error] 32-33: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 33-34: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 39-40: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 47-48: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
LanguageTool
apps/docs/content/docs/components/select.mdx
[grammar] ~9-~9: The plural noun “displays” cannot be used with the article “A”. Did you mean “A select display” or “select displays”?
Context: .../content/components/select"; # Select A select displays a collapsible list of options and allow...(A_NNS)
[uncategorized] ~122-~122: Did you mean: “By default,”?
Context: ...tContent} /> ### Custom Selector Icon By default the select uses achevron-downicon a...(BY_DEFAULT_COMMA)
[uncategorized] ~131-~131: You might be missing the article “the” here.
Context: ...f the icon. ### Without Scroll Shadow Select component uses the [ScrollShadow](/docs...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~176-~176: Did you mean: “By default,”?
Context: ...stomItems} /> ### Custom Render Value By default the select will render the selected ite...(BY_DEFAULT_COMMA)
[grammar] ~186-~186: The name of this game or TV show is spelled with an accent.
Context: ...e are using a custom hook to fetch the Pokemon API...(POKEMON)
[typographical] ~248-~248: It appears that a comma is missing.
Context: ...ing therenderValueproperty. In this example we are using the [Chip](/docs/component...(DURING_THAT_TIME_COMMA)
[uncategorized] ~260-~260: You might be missing the article “the” here.
Context: ...ect by using theclassNamesproperty. Select component also provides the [popoverPro...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~339-~339: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...pport for selecting multiple options. - Support for disabled options. - Support for sec...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~340-~340: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ions. - Support for disabled options. - Support for sections. - Labeling support for ac...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[typographical] ~359-~359: Consider adding a comma after ‘Usually’ for more clarity.
Context: ... | The children to render. Usually a list ofSelectItemand `SelectSecti...(RB_LY_COMMA)
[grammar] ~374-~374: In this case, the usual preposition with “side” is “on”, not “in”. Did you mean “on the left side”?
Context: ... | Element to be rendered in the left side of the select. ...(IN_ON_THE_RIGHT_HAND_SIDE)
[grammar] ~375-~375: In this case, the usual preposition with “side” is “on”, not “in”. Did you mean “on the right side”?
Context: ... | Element to be rendered in the right side of the select. ...(IN_ON_THE_RIGHT_HAND_SIDE)
apps/docs/content/docs/components/dropdown.mdx
[uncategorized] ~50-~50: Loose punctuation mark.
Context: ...ropdownItem } from "@nextui-org/react";, individual:import { Dropdown, ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~57-~57: Loose punctuation mark.
Context: ...downItem } from "@nextui-org/dropdown";`, }} /> ## Usage <CodeDemo title="Usa...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~66-~66: Possible missing article found.
Context: ...wnContent.usage} /> ### Dynamic items Dropdown follows the [Collection Components API]...(AI_HYDRA_LEO_MISSING_A)
[typographical] ~80-~80: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...important to have a unique key for each item, otherwise the disabled keys will not work. ### A...(THUS_SENTENCE)
[uncategorized] ~165-~165: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...m>` component works with frameworks and client side routers like [Next.js](https://nextjs.o...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[misspelling] ~246-~246: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...starts with the typed character if such an menu item exists. | <Spacer y={4...(EN_A_VS_AN)
[style] ~274-~274: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ection. - Support for disabled items. - Support for sections. - Complex item labeling s...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[typographical] ~290-~290: Consider adding a comma after ‘Usually’ for more clarity.
Context: ... | The children to render. Usually aDropdownTriggerandDropdownMenu...(RB_LY_COMMA)
[style] ~303-~303: This phrase is redundant. Consider using “outside”.
Context: ...ser interacts with the argument element outside of the dropdown ref, returntrueif `onC...(OUTSIDE_OF)
[typographical] ~355-~355: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...| The contents of the dropdown section. Usually a list ofDropdownItemcomponents. (s...(RB_LY_COMMA)
Additional comments not posted (13)
.changeset/chilled-worms-develop.md (2)
2-5: Patch updates listed correctly.The changeset correctly identifies the affected packages and specifies the type of change as a patch, which is appropriate given the nature of the changes (type fixes and enhancements).
8-8: Clear summary of changes.The summary "Fix onSelectionChange type incorrect" concisely captures the essence of the changes made in this PR.
packages/core/system-rsc/src/types.ts (2)
5-5: Import ofAriaSharedSelectionis appropriate.The import from
@react-types/sharedis necessary for extending theSharedSelectiontype, which aligns with the PR's objectives to correct the type definitions.
85-88: Definition ofSharedSelectiontype.The
SharedSelectiontype correctly extendsAriaSharedSelectionwith optionalanchorKeyandcurrentKey, which matches the new requirements for theonSelectionChangecallback across various components.packages/components/menu/src/use-menu.ts (2)
92-92: Addition ofonSelectionChangehandler is appropriate.The introduction of the
onSelectionChangehandler with the typeSharedSelectionin thePropsinterface correctly aligns with the PR's objectives to standardize the type definitions across components.
97-97: Correct omission ofonSelectionChangefromAriaMenuProps.Omitting
onSelectionChangefromAriaMenuPropswhile including it in the customUseMenuPropsensures that the custom handler is used, which supports the newSharedSelectiontype.packages/components/select/src/use-select.ts (1)
135-135: Addition ofonSelectionChangehandler is appropriate.The introduction of the
onSelectionChangehandler with the typeSharedSelectionin thePropsinterface correctly aligns with the PR's objectives to standardize the type definitions across components.apps/docs/content/docs/components/select.mdx (2)
362-364: Documentation update forselectedKeys,disabledKeys, anddefaultSelectedKeys.The documentation correctly updates the types for
selectedKeys,disabledKeys, anddefaultSelectedKeystoIterable<React.Key>, reflecting the changes made in the PR.
403-403: UpdatedonSelectionChangedocumentation.The updated documentation for
onSelectionChangeaccurately reflects the new callback signature, includinganchorKeyandcurrentKey, aligning with the changes made in the related components.apps/docs/content/docs/components/dropdown.mdx (4)
325-325: Type change forselectedKeysprop enhances flexibility.The update from
React.Key[]toIterable<React.Key>allows for more versatile collections of keys, supporting not just arrays but also other iterable types likeSetorMapkeys. This change is beneficial for developers looking to use different data structures.
326-326: Consistent type enhancement fordisabledKeysprop.Changing the type from
React.Key[]toIterable<React.Key>aligns this prop with the updatedselectedKeysprop, ensuring consistency across the component API. This change facilitates the use of various iterable types, enhancing flexibility.
327-327: Uniform type update fordefaultSelectedKeysprop.Updating the type from
React.Key[]toIterable<React.Key>aligns this prop with theselectedKeysanddisabledKeysprops, maintaining consistency and flexibility in the component's API. This uniform approach is beneficial for developers.
346-346: Enhanced functionality inonSelectionChangecallback.The updated type
(keys: "all" | Set<React.Key> & {anchorKey?: string; currentKey?: string})for theonSelectionChangecallback reflects the actual behavior of the component, providing developers with more detailed information about the selection context. This change improves the API's accuracy and usability.
Fixed 🫰 |
|
fixed |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- .changeset/chilled-worms-develop.md (1 hunks)
- packages/components/menu/src/use-menu.ts (2 hunks)
- packages/components/select/src/use-select.ts (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- .changeset/chilled-worms-develop.md
- packages/components/menu/src/use-menu.ts
- packages/components/select/src/use-select.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/components/select/src/use-select.ts (3 hunks)
Additional comments not posted (4)
packages/components/select/src/use-select.ts (4)
9-9: Import statement forSharedSelectionlooks good.The import statement for
SharedSelectionfrom@nextui-org/systemis correctly added.
132-135: Addition ofonSelectionChangeproperty toPropsinterface looks good.The
onSelectionChangeproperty is correctly defined as an optional function with the type(keys: SharedSelection) => void.
147-151: Update toUseSelectPropstype looks good.The
UseSelectPropstype correctly omitsonSelectionChangefromMultiSelectPropsand includes the newPropsinterface.
153-153: Changes touseSelectfunction look good.The
useSelectfunction correctly handles theonSelectionChangecallback with theSharedSelectiontype.
Closes #2512
📝 Description
Selectiontype add addtional key like as shown in the picture below⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
onSelectionChangehandling.Bug Fixes
Refactor
Documentation