Skip to content
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

Enforce type for children of Tabs component #6766

Closed
jonkoops opened this issue Jan 12, 2022 · 5 comments · Fixed by #6767
Closed

Enforce type for children of Tabs component #6766

jonkoops opened this issue Jan 12, 2022 · 5 comments · Fixed by #6767
Assignees
Labels
Breaking change 💥 this change requires a major release and has API changes.
Milestone

Comments

@jonkoops
Copy link
Contributor

The Tabs component can only accept children of the type of the Tab component. This is documented by a comment, but not enforced by TypeScript. To resolve this issue the correct type should be added so errors can be caught at compile time.

@jonkoops jonkoops changed the title Enfore type for children of Tabs component Enforce type for children of Tabs component Jan 12, 2022
@nicolethoen nicolethoen moved this to PR Review in PatternFly Issues Jan 12, 2022
@mcarrano mcarrano added this to the 2022.01 milestone Jan 14, 2022
Repository owner moved this from PR Review to Done in PatternFly Issues Jan 17, 2022
@nicolethoen
Copy link
Contributor

nicolethoen commented Jan 26, 2022

The updates made in #6767 caused a type error in OpenShift. For now, the update was reverted to unblock the current release.

../console-app/src/components/user-preferences/UserPreferencePage.tsx:137:17 - error TS2322: Type 'ReactElement<TabProps, string | JSXElementConstructor<any>>[]' is not assignable to type '(TabsChild | TabsChild[]) & ReactNode'.
  Type 'ReactElement<TabProps, string | JSXElementConstructor<any>>[]' is not assignable to type 'TabsChild[]'.
    Type 'ReactElement<TabProps, string | JSXElementConstructor<any>>' is not assignable to type 'TabsChild'.
      Type 'ReactElement<TabProps, string | JSXElementConstructor<any>>' is not assignable to type 'TabElement'.
        Type 'string | JSXElementConstructor<any>' is not assignable to type 'JSXElementConstructor<TabProps>'.
          Type 'string' is not assignable to type 'JSXElementConstructor<TabProps>'.

137                 {userPreferenceTabs}
                    ~~~~~~~~~~~~~~~~~~~~

  ../../node_modules/@patternfly/react-core/dist/esm/components/Tabs/Tabs.d.ts:14:5
    14     children: TabsChild | TabsChild[];
           ~~~~~~~~
    The expected type comes from property 'children' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Tabs> & Pick<Readonly<TabsProps> & Readonly<{ children?: ReactNode; }>, "children"> & Partial<...> & Partial<...>'

@nicolethoen nicolethoen reopened this Jan 26, 2022
@jonkoops
Copy link
Contributor Author

@nicolethoen Interesting, does the code there actually work? Seem like they are using strings as children where they would not be possible.

@mcarrano mcarrano removed this from the 2022.01 milestone Feb 8, 2022
@mcarrano mcarrano added the Breaking change 💥 this change requires a major release and has API changes. label Feb 8, 2022
@mcarrano mcarrano moved this from Done to Not started in PatternFly Issues Feb 8, 2022
@nicolethoen nicolethoen added this to the Breaking Changes milestone May 10, 2022
@nicolethoen nicolethoen moved this from Not started to Breaking change in PatternFly Issues Sep 23, 2022
@tlabaj tlabaj modified the milestones: Breaking Changes, 2022.15 Sep 27, 2022
@nicolethoen nicolethoen moved this from Breaking change to Not started in PatternFly Issues Oct 5, 2022
@nicolethoen
Copy link
Contributor

@tlabaj do we want to reopen the associated PR for this issue and merge it into the v5 branch?

@jonkoops
Copy link
Contributor Author

@nicolethoen I've re-opened this PR targeting the v5 branch instead.

@nicolethoen
Copy link
Contributor

Closed by #8217

Repository owner moved this from Not started to Done in PatternFly Issues Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking change 💥 this change requires a major release and has API changes.
Projects
Archived in project
4 participants