Typescript-Migration: @storybook/addons #5018
Conversation
kroeder
added
ci: do not merge
api: addons
maintenance
kroeder
requested review from
alterx,
igor-dv,
thani-sh,
ndelangen and
theinterned
as code owners
Codecov Report
@@ Coverage Diff @@
## next #5018 +/- ##
==========================================
+ Coverage 35.05% 35.09% +0.03%
==========================================
Files 593 594 +1
Lines 7348 7358 +10
Branches 1000 1002 +2
==========================================
+ Hits 2576 2582 +6
- Misses 4259 4265 +6
+ Partials 513 511 -2
Continue to review full report at Codecov.
|
| @@ -9,13 +9,15 @@ | |||
| "noImplicitAny": true, | |||
| "jsx": "react", | |||
There was a problem hiding this comment.
jsx allows react or react-native
Should there be a check for special react native build steps?
like if(buildReactNative) { tsconfig.jsx = 'react-native'; } ?
| babelify({ errorCallback: () => logError('js', packageJson) }); | ||
| removeTsFromDist(); | ||
| tscfy({ errorCallback: () => logError('ts', packageJson) }); | ||
| } |
There was a problem hiding this comment.
Pure ts projects may cause unexpected behavior when it runs through babelify as well
In my case babelify copied tsx files into dist
| @@ -18,7 +18,7 @@ | |||
| "bootstrap": "node ./scripts/bootstrap.js", | |||
| "bootstrap:crna-kitchen-sink": "npm --prefix examples-native/crna-kitchen-sink install", | |||
| "bootstrap:docs": "yarn install --cwd docs", | |||
| "build-packs": "lerna exec --scope '@storybook/*' --parallel -- \\$LERNA_ROOT_PATH/scripts/build-pack.sh \\$LERNA_ROOT_PATH/packs", | |||
| "build-packs": "lerna exec --scope '@storybook/*' -- \\$LERNA_ROOT_PATH/scripts/build-pack.sh \\$LERNA_ROOT_PATH/packs", | |||
There was a problem hiding this comment.
I had to remove --parallel in order to fix native-smoke-test
The ts build of addons always failed because it seemed that channels wasn't build but addons relied on channels so it could not build as well
I found this lerna/lerna#1681 and thought "give it a shot", though I'm not sure if this is okay? I spent a lot of time on fixing this, if this is not the right fix I definitely need help for another way.
| @@ -18,7 +18,7 @@ | |||
| "bootstrap": "node ./scripts/bootstrap.js", | |||
| "bootstrap:crna-kitchen-sink": "npm --prefix examples-native/crna-kitchen-sink install", | |||
| "bootstrap:docs": "yarn install --cwd docs", | |||
| "build-packs": "lerna exec --scope '@storybook/*' --parallel -- \\$LERNA_ROOT_PATH/scripts/build-pack.sh \\$LERNA_ROOT_PATH/packs", | |||
| "build-packs": "lerna exec --scope '@storybook/*' -- \\$LERNA_ROOT_PATH/scripts/build-pack.sh \\$LERNA_ROOT_PATH/packs", | |||
| settings: WrapperSettings | ||
| ) => any; | ||
|
|
||
| type MakeDecoratorResult = (...args: any) => any; |
There was a problem hiding this comment.
I believe this line wasn't correctly typed actually... It's not simply saying there is no type (any) but TSLint should point out an error saying args must be an array, so it should be any[].
Issue: N/A
What I did
Working on the TS-Migration with @ndelangen and @igor-dv