-
Notifications
You must be signed in to change notification settings - Fork 672
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
WIP: refactor(gatsby-plugin-theme-ui): add typescript #705
Conversation
"files": [ | ||
"src/**/*.js", | ||
"/*.js" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pretty unconventional way to use TS, but I'm not sure if it's worse than copying package.json and README to dist and modifying lerna.json.
"scripts": { | ||
"build": "tsc", | ||
"prepare": "yarn build || true", | ||
"test": "tsc --noEmit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could get executed in root package.json (jest && lerna run test
)
When a contributor opens the monorepo in his editor, he'll see TypeScript errors on all JSX usages in tests, because the test files may not be included in package tsconfigs (gatsby-plugin-theme-ui). Adding `jsx: react` to the root tsconfig won't hurt.
07cf829
to
9d6d8dd
Compare
@@ -0,0 +1,365 @@ | |||
import * as React from 'react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #757 (comment)
@@ -41,7 +41,9 @@ | |||
"ts-jest": "^25.2.0", | |||
"typescript": "^3.8.2" | |||
}, | |||
"resolutions": {}, | |||
"resolutions": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
described in #757 (comment)
@@ -42,7 +42,8 @@ | |||
"typescript": "^3.8.2" | |||
}, | |||
"resolutions": { | |||
"@types/vfile-message": "1.0.1" | |||
"@types/vfile-message": "1.0.1", | |||
"@types/react": "16.9.23" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple versions of @types/react
in node_modules
lead to a lot of suffering (global namespace conflicts).
Marked this as draft now. It needs to be updated. |
Ah! Sorry we're doing some updates to the Gatsby plugin -- since this relies on the shadowing API in Gatsby, I'd say let's hold off on converting this plugin and other Gatsby themes in the repo until later, since there are other things to consider. Currently these packages aren't compiled at all and Gatsby consumes them directly... |
Part of #668.
Needs types for
theme-ui
.Results of
npm pack
:gatsby-plugin-theme-ui-v0.3.0.zip