Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* Publish - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] * [NO-TICKET] Remove a missed withTranslation instance (#1515) * Remove a missed withTranslation instance We don't use the HOC anymore * Update tests and snapshots * Actually, the previous snapshots were more informative * [WNMGDS-1344][WNMGDS-1345] Fix Button types (#1532) * Working on fixing the Button prop types When coming across instances of `<Button>` in downstream apps, TypeScript wasn't able to determine which type of the intersection type `type ButtonProps = CommonButtonProps<ButtonComponent> | LinkButtonProps | ButtonButtonProps` was at play, so it would complain that props like `href` and `target` did not exist on `CommonButtonProps<ButtonComponent>` when it should have been using `LinkButtonProps`. The fix is to make the `Button` component itself a generic type, showing TypeScript which part of it is variable and it has to narrow down. This, I think, is also a little clearer and easier to follow than the big intersection type. It's either a regular html element or it's a `CustomButtonComponentType` (a React component), and the intrinsic HTML props are applied conditionally. There are a couple more places where I have to update references to ButtonProps in our code * Update the DrawerToggle module, but it still throws errors * Converted Button to FC but will revert I thought the generated type definition would drop the `: JSXElement` from the render function, but it didn't. The problem I'm now getting in downstream apps is that they can use any value they want for attributes like `size`, which we define on Button as having a specific set of allowable values * Define the Link props for testing the type checking * Remove default prop values to simplify discriminating types * Fix button tests and update HelpDrawerToggle * Try omitting href from intrinsic props - doesn't work * Don't use auto Merge utility type - that worked?! I suspect it could have to do with the 'ref' value actually... * Fix errors in DrawerToggle tests * Clean up and simplify * Figured out how to narrow the `OtherTypeButtonProps` The significance of this is that sometimes instances of Button even though they should be identified as having `LinkTypeButtonProps` (where `href` is strictly required)—like when explicitly passing `component='a'`—TypeScript seemed to be thinking it was the more generic `OtherTypeButtonProps<T>`. I _think_ this is working now. Error that flags a missing `href` prop if the component has an explicit `component='a'` prop: ``` packages/design-system/src/components/Button/Button.test.tsx:36:8 - error TS2322: Type '{ component: "a"; target: "_blank"; type: string; children: string; }' is not assignable to type 'IntrinsicAttributes & ButtonProps<"a">'. Property 'href' is missing in type '{ component: "a"; target: "_blank"; type: string; children: string; }' but required in type 'LinkTypeButtonProps'. 36 <Button ~~~~~~ packages/design-system/src/components/Button/Button.tsx:56:3 56 href: string; ~~~~ 'href' is declared here.`` ``` Note that it is correctly identifying this as a `LinkTypeButtonProps`. Before this change, no error was thrown for a missing `href`, but in a similar situation where we explicitly pass a `component='a'` prop but give an _invalid_ value for `href`, we'd get the following error which illustrates TypeScript's incorrect inference about the type of the component props: ``` packages/design-system/src/components/Button/Button.test.tsx:36:8 - error TS2322: Type '{ component: "a"; href: { hello: string; }; target: "_blank"; type: string; children: string; }' is not assignable to type 'IntrinsicAttributes & ButtonProps<"a">'. Type '{ component: "a"; href: { hello: string; }; target: "_blank"; type: string; children: string; }' is not assignable to type 'OtherTypeButtonProps<"a">'. Types of property 'href' are incompatible. Type '{ hello: string; }' is not assignable to type 'null'. 36 <Button ~~~~~~ ``` Note that it thinks our button has `OtherTypeButtonProps<"a">` props. We don't want that more generic type to be used when the more specific `LinkTypeButtonProps` should be used!! * Fix other bug about "secondary" not being valid variant in hc.gov * Match the button attribute behavior of before except without default "type" Buttons don't need the default "button" type anyway * Simplify Button types (#1538) * Simplify types, trading accuracy for simplicity These types are more permissive than before but contain no intersections or conditional types * May need to keep CommonButtonProps generic in order to infer T * Remove redundant type * Add the `type` default back in Need to do it the functional-component way, and also need to move the destructuring assignment into the function's call signature in order for the prop-table generator to pick up on that default value * Update snapshots after adding the default `type` back in * Fix problems Sarah identified * [WNMGDS-1344][WNMGDS-1345] Revert button types (#1545) * Revert back to the simplest types without support for custom component * HTMLAttributes didn't actually contain all attributes of anchors like `target` * [NO-TICKET] Deprecate Button's "component" prop (#1546) * Add deprecation docs and warning for Button's `component` prop * Update unit tests and story * Publish - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] * updating inputRef typing on Button component (#1553) * Publish - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] * merging deletion of svg icons commit 68a62c8 * Publish - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] * Publish - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] * Make the StepList props with default values optional * Publish - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] - @cmsgov/[email protected] Co-authored-by: Anna Kammeyer <[email protected]> Co-authored-by: Patrick Wolfert <[email protected]> Co-authored-by: Anna Scheuler Kammeyer <[email protected]>
- Loading branch information
4 people
authored
Jan 31, 2022
1 parent
4ef0890
commit 8cafc13