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

Add TypeScript types to @carbon/react components #12513

Closed
tay1orjones opened this issue Nov 7, 2022 · 28 comments
Closed

Add TypeScript types to @carbon/react components #12513

tay1orjones opened this issue Nov 7, 2022 · 28 comments
Labels
afrohacks See https://ibm.biz/afrohacks-hackathon area: typescript hacktoberfest See https://hacktoberfest.com/ needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. planning: umbrella Umbrella issues, surfaced in Projects views role: dev 🤖 status: help wanted 👐
Milestone

Comments

@tay1orjones
Copy link
Member

tay1orjones commented Nov 7, 2022

This issue is to track the implementation of TypeScript definitions for components exported from @carbon/react.

The lists below organize all exports from the package that need to have types added into three categories: high, medium, and low priority. There is a project board for this effort as well.

How to contribute

  • If you'd like to contribute, please comment in the component issue stating your intent to work on an issue.
  • If that person has not submitted a PR within 10 days, consider it available again.

Thanks so much for assisting with this huge effort! 🙏

Every checklist item below needs the following:

  1. Add component types for all components listed in the title by following the adding component types documentation
  2. Submit a pull request for the changes
    • Please keep pull requests as small as possible
    • Avoid adding additional components to a single PR unless necessary
    • Link to close the related issue, e.g. Closes #12513

REF #16360

High priority

DataTable

Grid

Inputs

Medium priority

Low priority

UIShell

UIShell Header

UIShell SideNav

Menu-based components

Hooks

Feature flagged components

@tay1orjones tay1orjones changed the title Provide TypeScript types for components Add TypeScript types to @carbon/react components Nov 7, 2022
@tay1orjones tay1orjones added planning: umbrella Umbrella issues, surfaced in Projects views role: dev 🤖 needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. area: typescript labels Nov 7, 2022
@tay1orjones tay1orjones moved this from Todo to In Progress in Initial TypeScript Support Nov 7, 2022
@tay1orjones tay1orjones added status: help wanted 👐 hacktoberfest See https://hacktoberfest.com/ labels Nov 7, 2022
@HiggsGaama

This comment was marked as resolved.

@tw15egan

This comment was marked as resolved.

jpsorensen added a commit to jpsorensen/carbon that referenced this issue Dec 14, 2022
Convert existing propType definitions to Typescript annotations on the
NumberInputSkeleton component. This is part of a broader effort to add
Typescript annotations to components, tracked in carbon-design-system#12513. Closes carbon-design-system#12549.

Type annotation changes only; no breaking feature changes.
jpsorensen added a commit to jpsorensen/carbon that referenced this issue Dec 19, 2022
Convert existing propType definitions to Typescript annotations on the
NumberInput component. This is part of a broader effort to add
Typescript annotations to components, tracked in carbon-design-system#12513. Closes carbon-design-system#12548.

Type annotation changes only; no breaking feature changes.
jpsorensen added a commit to jpsorensen/carbon that referenced this issue Jan 19, 2023
Convert existing propType definitions to Typescript annotations on the
TableSelectAll component. This is part of a broader effort to add
Typescript annotations to components, tracked in carbon-design-system#12513. Closes carbon-design-system#12530.

Type annotation changes only; no breaking feature changes.
@lewandom

This comment was marked as off-topic.

@tay1orjones
Copy link
Member Author

@lewandom Awesome! Please comment on the individual component issues so interest/assignments are visible there in context

kodiakhq bot added a commit that referenced this issue Jan 19, 2023
Convert existing propType definitions to Typescript annotations on the
NumberInputSkeleton component. This is part of a broader effort to add
Typescript annotations to components, tracked in #12513. Closes #12549.

Type annotation changes only; no breaking feature changes.

Co-authored-by: Taylor Jones <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Alessandra Davila <[email protected]>
jpsorensen added a commit to jpsorensen/carbon that referenced this issue Jan 22, 2023
Convert existing propType definitions to Typescript annotations on the
NumberInput component. This is part of a broader effort to add
Typescript annotations to components, tracked in carbon-design-system#12513. Closes carbon-design-system#12548.

Type annotation changes only; no breaking feature changes.
kodiakhq bot pushed a commit that referenced this issue Jan 28, 2023
* refactor(TableSelectAll): add typescript annotations

Convert existing propType definitions to Typescript annotations on the
TableSelectAll component. This is part of a broader effort to add
Typescript annotations to components, tracked in #12513. Closes #12530.

Type annotation changes only; no breaking feature changes.

* fix(typescript): fix InlineCheckbox property types

Because InlineCheckbox does not have type definitions, it gets its input
property type definitions from React.forwardRef, and so does not allow extra
properties. This results in errors like:
Property 'ariaLabel' does not exist on type 'IntrinsicAttributes &
RefAttributes<any>'.
when we adopt TS in files that use InlineCheckbox. Since InlineCheckbox is
internal-only, policy is not to add types but to mark as type: any.

* fix(typescript): fix DataTable exports tests

DataTable has a set of tests which check that all files in the directory (with
some specific exceptions) export a default component with the same name as the
file. This test got the expected name of the component by stripping ".js" from
the filename. This won't work for Typescript files, which have a different
extension. Stripped off the extension regardless of what it was instead.
@jfroffice
Copy link

any updates on this MR.

Is it possible at least to merge the migrations that have been already done.

It would be helpful.

@imp-dance
Copy link
Contributor

imp-dance commented May 27, 2024

Does anyone have experience using Carbon with Typescript in it's current state? We would love to upgrade at work, but this is quite the major blocker for us. It does seem like there's very few components left though. I'll try to punch out a few more as soon as I have the time.

Just wanted to give an update here for anyone considering upgrading.

Updating was relatively painless. I'd definitely recommend anyone trying this to use the migration tools provided. After running these, we updated our SASS (which was a bit of a pain just because we've set it up unconventionally), then we went through the components that have changed (surprisingly few) and either wrapped them to not break our own behaviour, or just updated the interface wherever possible. After finally adding a few missing type definitions in a index.d.ts file everything is up and running again!

The missing type definitions should not be a blocker, there are so few left and it's pretty straight forward to just add your own type definitions for whatever is missing. Example:

// index.d.ts

type OverflowMenuProps = {...}; // use Storybook for reference

declare module "@carbon/react" {
   export * from "@carbon/react";
   export const OverflowMenu: (props: OverflowMenuProps) => React.ReactNode;
   // ....
}

@2nikhiltom
Copy link
Contributor

Hey @Agile-fox

CheckboxGroup component seems to be missing from the list 😄

Good catch, it will be tracked via this issue

@sstrubberg sstrubberg moved this to 🏗 In Progress in Design System Jul 19, 2024
@sstrubberg sstrubberg added this to the 2024 Q3 milestone Jul 22, 2024
@sstrubberg sstrubberg moved this to In Progress in Roadmap (deprecated) Jul 25, 2024
wkeese added a commit to wkeese/carbon that referenced this issue Jul 29, 2024
Add proper types for parameter to translateWithId() methods.

Each component's translateWithId() method can only take a certain
list of keys, not any string.  Update the Typescript to notate this.

For historical reasons most of the existing translation keys are
declared in a convoluted way.  For example:

export const translationIds = {
  'increment.number': 'increment.number',
  'decrement.number': 'decrement.number',
};

type TranslationKey = keyof typeof translationIds;

The simpler way is just:

type TranslationKey = 'increment.number' |  'decrement.number’;

I didn’t update that in this PR, but it’s something to consider for the future.
Note that TableToolbarSearch.tsx and MultiSelect.tsx don’t suffer from this
problem.

Also note that many components declare translateWithId() directly, whereas
others do it by extending InternationalProps.  I guess all components with
translation should extend InternationalProps, but I didn’t update that in this
commit either.

Refs carbon-design-system#12513.
wkeese added a commit to wkeese/carbon that referenced this issue Jul 29, 2024
Add proper types for parameter to translateWithId() methods.

Each component's translateWithId() method can only take a certain
list of keys, not any string.  Update the Typescript to notate this.

For historical reasons most of the existing translation keys are
declared in a convoluted way.  For example:

export const translationIds = {
  'increment.number': 'increment.number',
  'decrement.number': 'decrement.number',
};

type TranslationKey = keyof typeof translationIds;

The simpler way is just:

type TranslationKey = 'increment.number' |  'decrement.number’;

I didn’t update that in this PR, but it’s something to consider for the future.
Note that TableToolbarSearch.tsx and MultiSelect.tsx don’t suffer from this
problem.

Refs carbon-design-system#12513.
github-merge-queue bot pushed a commit that referenced this issue Jul 31, 2024
* fix: translateWithId() parameter types

Add proper types for parameter to translateWithId() methods.

Each component's translateWithId() method can only take a certain
list of keys, not any string.  Update the Typescript to notate this.

For historical reasons most of the existing translation keys are
declared in a convoluted way.  For example:

export const translationIds = {
  'increment.number': 'increment.number',
  'decrement.number': 'decrement.number',
};

type TranslationKey = keyof typeof translationIds;

The simpler way is just:

type TranslationKey = 'increment.number' |  'decrement.number’;

I didn’t update that in this PR, but it’s something to consider for the future.
Note that TableToolbarSearch.tsx and MultiSelect.tsx don’t suffer from this
problem.

Refs #12513.

* chore: extend InternationalProps instead of declaring translateWithId()

Refs #12513.

* chore: rename InternationalProps

---------

Co-authored-by: Taylor Jones <[email protected]>
@Mechons
Copy link

Mechons commented Aug 5, 2024

Hi I'm getting this error while importing carbon component in my tsx file .

image

I'm using carbon/ react version "@carbon/react": "^1.33.0",.

please recommend how to resolve this

@jfroffice @tay1orjones

@github-project-automation github-project-automation bot moved this from In Progress to Done in Roadmap (deprecated) Aug 27, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In Progress to ✅ Done in Design System Aug 27, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Initial TypeScript Support Aug 27, 2024
@tay1orjones
Copy link
Member Author

The scope that was initially planned here is done! There are some follow-on items yet to be tackled:

For any bugs related to existing types, please open a new issue

@github-project-automation github-project-automation bot moved this to Triage in Roadmap Sep 14, 2024
@sstrubberg sstrubberg moved this from Triage to Completed 🚢 in Roadmap Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
afrohacks See https://ibm.biz/afrohacks-hackathon area: typescript hacktoberfest See https://hacktoberfest.com/ needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. planning: umbrella Umbrella issues, surfaced in Projects views role: dev 🤖 status: help wanted 👐
Projects
Archived in project
Status: Done
Archived in project
Development

No branches or pull requests