chore(carbon__icons-react): ship TypeScript definitions #12034
Conversation
metonym
force-pushed
the
@carbon__icons-react-types
branch
from
cfea21e
to
1abec37
Compare
✅ Deploy Preview for carbon-components-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for carbon-components-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| { | ||
| "compilerOptions": { | ||
| "lib": ["es2015.iterable", "es2015.generator", "es5"], | ||
| "moduleResolution": "node", | ||
|
|
||
| "declaration": true, | ||
| "declarationMap": true, | ||
| "sourceMap": true, | ||
| "composite": true, | ||
| "noEmitOnError": true, | ||
|
|
||
| "strictNullChecks": true, | ||
| "noImplicitAny": true, | ||
| "noImplicitThis": true, | ||
| "strictPropertyInitialization": true, | ||
| "noUnusedLocals": true, | ||
| "noUnusedParameters": true, | ||
|
|
||
| "alwaysStrict": true, | ||
| "preserveConstEnums": true, | ||
|
|
||
| "jsx": "preserve", | ||
|
|
||
| "types": [] | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure this is the PR in which we want to define the overall monorepo's tsconfig file. I'd be more comfortable with each respective package defining their own for now until we have a global baseline that is defined by the TS working group.
Separately, It might be worth generating the basic tsconfig files using the tsc --init command since it outputs all (read: most) possible settings and gives nice context to each of them. Then from there, we can make changes to the configs above and beyond the defaults as needed.
| @@ -26,6 +26,7 @@ | |||
| "sync": "carbon-cli sync", | |||
| "test": "cross-env BABEL_ENV=test jest", | |||
| "test:e2e": "cross-env BABEL_ENV=test jest -c jest.e2e.config.js", | |||
| "typecheck": "tsc -b", | |||
There was a problem hiding this comment.
This script name implies type-checking and not necessarily building. Would it make more sense for this to be tsc --noEmit?
|
|
||
| import * as React from 'react'; | ||
|
|
||
| type IconSize = 16 | 20 | 24 | 32 | '16' | '20' | '24' | '32' | number | string; |
There was a problem hiding this comment.
There's an outstanding comment relating to this that I think needs some discussion. You can ignore my comment on it because the answer to what I asked over there is "yes", but the comment about IDE autocompletion is worth exploring further.
Also, I'm not sure why strings are allowable on this prop. @tay1orjones Do react icons allow you to specify string versions of the number sizes?
There was a problem hiding this comment.
@jdharvey-ibm Yes, both strings or number literals are allowed
metonym
force-pushed
the
@carbon__icons-react-types
branch
from
1abec37
to
67d9498
Compare
Closes #11317
This PR is based on #11533. I simply copied and rebased the PR on
main.Currently,
@types/carbon__icons-reacthas to be manually updated for TypeScript support.The goal for this PR is to better support TypeScript for the
@carbon/icons-reactpackage without rewriting it completely. By generating and shipping definitions with each release,@types/carbon__icons-reactcan be removed.Related discussions in
DefinitelyTyped:Changelog
New