-
Notifications
You must be signed in to change notification settings - Fork 150
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
chore: update flow to 0.147.0 #2855
Conversation
4783442
to
3eafc3d
Compare
Size Change: +7.98 kB (+2%) Total Size: 480 kB
ℹ️ View Unchanged
|
a340176
to
0f73999
Compare
Had to update styled-components exports, because it had many errors when we use in interpolation reference to a different component, like:
changed from:
to:
suppressed A lot of things were fixed by codemod from Read more here: |
0f73999
to
d674959
Compare
df6c72f
to
93d7672
Compare
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.
I reviewed what I concluded were manually changed files, I'll have to assume that the rest of the ~800 files are fine, I hope that's ok 😅
@@ -1,4 +1,4 @@ | |||
// @flow | |||
// @noflow |
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 looks like a good opportunity to finally scope this ESLint rule because keep having to do this is getting ridiculous 😄 Something like this might work, I haven't tried:
{
files: [
"packages/orbit-components/{src,es,lib}/**/*.js",
"packages/orbit-design-tokens/{src,lib}/**/*.js",
"*.js.flow",
],
rules: {
"flowtype/require-valid-file-annotation": ["error", "always"],
},
},
I think those are the only places we're using Flow. Then remove flowtype/require-valid-file-annotation
from the override that targets all *.js
and *.js.flow
files.
Then you can automatically remove all other file annotations, even if they are // @flow
because I doubt they were doing much for Node files anyway.
Yep, it's quite a lot to review :D thanks |
372dcdc
to
20524e9
Compare
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.
As far as I'm concerned, this is good 👍
20524e9
to
f9d9e74
Compare
f9d9e74
to
bf020b0
Compare
Got more than 1300 errors, managed to fix them all. Tried to type it where it's really necessary to have, suppressed with
FlowFixMe
, where it seemed to be not so important (tests, stories, defaultProps problem), as soon as we plan to move the entire codebase to typescript, I think we don't have to waste time on perfect flow types. Read more in the comment below.Orbit.kiwi: https://orbit-docs-chore-flow-update.surge.sh
Storybook: https://orbit-chore-flow-update.surge.sh