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

[Bug]: Recent TypeScript updates in v1.22.0 cause a bunch of typescript import errors for components that haven't been converted to typescript yet #13091

Open
2 tasks done
fbarroso24 opened this issue Feb 3, 2023 · 8 comments
Assignees

Comments

@fbarroso24
Copy link

Package

@carbon/react

Browser

Chrome, Edge

Package version

@carbon/react: "1.22.0"

React version

react: "16.14.0"

Description

Just upgraded to @carbon/react: "1.22.0" which introduced some typescript updates and now all the carbon components that do not have .ts files in their component directories (e.g. a majority of them) fail to load from @carbon/react. In the below screenshot you'll see that both FormContext & FluidForm components import OK as they have .ts files in their directory but the other components that do not all error out.

This was discussed in more detail in https://ibm-industrysolutions.slack.com/archives/C03C8VASVED/p1675447829358449.

A temporary workaround is to update your d.ts file to include declare module "@carbon/react" (see 1 from screenshot below). However, anything that uses React.forwardRef, React.createRef or React.RefObject that references a Carbon component will still have issues (see 2 from screenshot below). The workaround for this is to add a @ts-ignore tag above the line in question.
image

The downsides of having to do the above workarounds is that components will neither autocomplete nor autoimport which is a big pain and all components that are imported by @carbon/react are treated as type any.

A suggested fix in the slack thread was to provide a index.d.ts that has unconverted components typed as any so that people can still import them even though they are not converted yet.

Suggested Severity

Severity 1 = Must be fixed ASAP. The response must be swift. Someone from the team must drop all current work and be immediately reassigned to address the issue.

Reproduction/example

See slack thread discussed in more detail in https://ibm-industrysolutions.slack.com/archives/C03C8VASVED/p1675447829358449

Steps to reproduce

See slack thread discussed in https://ibm-industrysolutions.slack.com/archives/C03C8VASVED/p1675447829358449

Code of Conduct

@tay1orjones
Copy link
Member

Hey thanks for reporting this! I appreciate you surfacing it to the typescript working group internally. I've also added it to the TypeScript Adoption project for this repo.

A suggested fix in the slack thread was to provide a index.d.ts that has unconverted components typed as any so that people can still import them even though they are not converted yet.

This seems reasonable and could be a good step forward to eliminate this issue.

I've classified this a severity: 3 https://ibm.biz/carbon-severity based on the following workaround you mention:

The workaround for this is to add a @ts-ignore tag above the line in question
The downsides ... is that components will neither autocomplete nor autoimport

I fully agree this is not ideal and not having autocomplete/import is a pain, but not having a hard blocker puts this lower on our list. It would be great if someone from the TypeScript working group could dig into this.

@guilhermeavanci
Copy link

+1

I had the same problem here. Thanks for the workaround btw!

I hope the fix comes soon. I'm having a lot of trouble here not being able to make imports and autocomplete, it's also hard to keep track of what exactly the library exports.

@vlad-saling
Copy link

I'd love a fix for this. While I understand, that this is rather an inconvenience and not "really" breaking anything it affects daily dev QoL and prolongs certain efforts (e.g. looking up undocumented props).

@mastry
Copy link

mastry commented Aug 7, 2023

This should be a higher priority. I'm currently going through an evaluation of several design systems for our next project and this issue prevents me from even considering Carbon. That's very disappointing since I think Carbon would otherwise be a great fit for us.

@dwwr
Copy link

dwwr commented Jan 4, 2024

Happy new year! Agreed this should be higher priority. Or remove the "Typescript support" Icon from the NPM.

Screen Shot 2024-01-04 at 11 07 10 AM

@jfroffice
Copy link

any update on this issue ?

@rwibm
Copy link

rwibm commented Mar 5, 2024

@tay1orjones Can we get an update here please? This issue is now over a year old with no meaningful update from the carbon team.

This needs to be a higher priority; as it stands Carbon claims to have TS support, when it doesn't, not properly anyway. The definitions that are there are wrong in some cases, and there are still several missing entirely.

We started migrating our codebases to TypeScript as it was claimed to be supported by carbon, but we're now hitting brick walls due to this issue.

Any sort of update on efforts to resolve this would be greatly appreciated.

@tay1orjones
Copy link
Member

Thanks for probing an update! We've always stated typings are provided on an as-is basis as we work through the incremental adoption strategy that we agreed on with the typescript working group.

My understanding of this issue is that once every export has a .ts(x) file this problem will self-resolve. I know this hasn't been an ideal situation, but the good news is there has been significant progress on adding types for all components. There's now just ~12 components remaining. Our team is definitely aware of the need to finish this up and we're evaluating when we could help tackle the remaining work. The fastest way to resolve this though is with your help. If you find definitions are wrong or missing, we'd very much welcome a PR to help fix and get things where they need to be 💙

About the TS npm badge - we haven't added the typings or types key to @carbon/react's package.json but it unfortunately still displays the badge. I assume because we're publishing a definition file. To my knowledge there is no way for us to control it being displayed or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏗 In Progress
Development

No branches or pull requests

10 participants