Skip to content

fix(types): accordion, relax children type#5033

Merged
chasestarr merged 2 commits intouber:masterfrom
zxbodya:accordion-children-type-fix
Jul 15, 2022
Merged

fix(types): accordion, relax children type#5033
chasestarr merged 2 commits intouber:masterfrom
zxbodya:accordion-children-type-fix

Conversation

@zxbodya
Copy link
Contributor

@zxbodya zxbodya commented Jul 15, 2022

Description

Current type does not allow to pass children like this:

    <Accordion>
      {[
          <Panel key="1">1</Panel>,
          <Panel key="2">2</Panel>
      ]}
      <Panel key="3">3</Panel>
    </Accordion>

Making the type less strict (ReactNode instead of only ReactElement), also adding support for primitive values

Scope

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 15, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit fe1589a:

Sandbox Source
Basic usage Configuration

Comment on lines -26 to +37
const key = child.key || String(index);
return React.cloneElement(child, {
disabled: child.props.disabled || disabled,
let normalizedChild =
typeof child === 'object' ? (
(child as // skip {} from type definition
| React.ReactElement<any, string | React.JSXElementConstructor<any>>
| React.ReactPortal)
) : (
// if primitive value - wrap it in a fragment
<>{child}</>
);
const key = normalizedChild.key || String(index);
return React.cloneElement(normalizedChild, {
disabled: normalizedChild.props.disabled || disabled,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as an option we can just ignore "unexpected elements", like this:

{React.Children.map(children, (child: React.ReactElement | React.ReactPortal, index) => {
        if (!child.props) return;

return React.cloneElement(child, {
disabled: child.props.disabled || disabled,
let normalizedChild =
typeof child === 'object' ? (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will ReactIs.isElement(child) handle the type cast?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that would allow to remove typecast.

updated to use it, also added check for isPortal - that looks like another allowed case here

@chasestarr chasestarr merged commit eaceb83 into uber:master Jul 15, 2022
@chasestarr chasestarr deleted the accordion-children-type-fix branch July 15, 2022 23:23
andriuss pushed a commit to andriuss/baseweb that referenced this pull request Aug 2, 2022
* fix(types): accordion - relax children type

* fix: use react-is
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants