react icon indicating copy to clipboard operation
react copied to clipboard

Inaccurate type definitions for Button variant prop

Open rezrah opened this issue 2 years ago • 1 comments

Describe the bug Type definitions for Button are not being exported correctly in the transpiled output.

E.g:

variant is being reported as a generic string type, rather than the VariantType union it should report.

This may be happening with other props, but hasn't been verified.

Due to this incorrect typing, the migration from the v34 Button to v35 becomes challenging as the v34 Button also had a variant prop that was used for size, so the TypeScript compiler assumes the size was valid and shows the following error (rather than one related to invalid discriminated unions).

<Button
    variant="small"
    onClick={() => onCreate(minimumStartDate, defaultDuration)}
    leadingIcon={PlusIcon}
    {...testIdProps('add-default-iteration-button')}
  >
    Add iteration
  </Button>

leads to this error:

Error: Uncaught [TypeError: Cannot convert undefined or null to object]

This can also be reproduced on StackBlitz:

To Reproduce Steps to reproduce the behavior:

  1. Go to StackBlitz https://stackblitz.com/edit/primer-react-v35?file=index.tsx
  2. Hover over the variant prop on Button
  3. See error button props

Expected behavior variant should show 'default' | 'primary' | 'invisible' | 'danger' | 'outline'

Additional context Possibly related? https://github.com/primer/react/issues/1948

rezrah avatar Mar 14 '22 10:03 rezrah

👋🏻 @pksjce we're doing some board cleanup and wanted to check in on this one. Is it on your radar / still in progress?

lesliecdubs avatar Apr 12 '22 16:04 lesliecdubs

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

github-actions[bot] avatar Oct 09 '22 17:10 github-actions[bot]

This affects all props on Button other than the generic indexed properties for string, number and symbol. None of the other props are type-checked or offered up by language server assistance.

jclem avatar Oct 17 '22 21:10 jclem

I believe https://github.com/primer/react/pull/2445 fixes this

mattcosta7 avatar Oct 21 '22 11:10 mattcosta7