Move typings for @storybook/react to @types package

[joscha]

Issue: #1166

• Hold off merging until https://www.npmjs.com/package/@types/storybook__react is available.
• Update docs

[codecov]

Codecov Report

Merging #1199 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1199   +/-   ##
=======================================
  Coverage   13.72%   13.72%           
=======================================
  Files         207      207           
  Lines        4611     4611           
  Branches      517      517           
=======================================
  Hits          633      633           
+ Misses       3529     3525    -4     
- Partials      449      453    +4

Impacted Files	Coverage Δ
.../src/manager/containers/CommentsPanel/dataStore.js	34.78% <0%> (ø)	⬆️
addons/knobs/src/components/PropField.js	10.86% <0%> (ø)	⬆️
app/react/src/client/preview/error_display.js	0% <0%> (ø)	⬆️
addons/knobs/src/components/types/Color.js	8% <0%> (ø)	⬆️
.../ui/src/modules/ui/components/left_panel/header.js	27.58% <0%> (ø)	⬆️
...rc/modules/ui/components/left_panel/text_filter.js	33.33% <0%> (ø)	⬆️
lib/ui/src/modules/ui/configs/init_panels.js	25% <0%> (ø)	⬆️
app/react/src/server/babel_config.js	44.82% <0%> (ø)	⬆️
app/react/src/client/preview/config_api.js	0% <0%> (ø)	⬆️
...react-native/src/manager/components/PreviewHelp.js	0% <0%> (ø)	⬆️
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 013e574...f2c0f1a. Read the comment docs.

[shilman]

@joscha this is awesome!! mind updating the docs as well and including that in the PR?

[ndelangen]

Thanks @joscha !

[joscha]

Docs updated in 5013dda - let's see how we go with this change and then we can update all the other packages.

[shilman]

Thanks so much @joscha . Going to test it out and then merge. Super excited about this!

[shilman]

@joscha worked like a charm. fabulous! 🌈 🦄 🌟

[joscha]

@joscha worked like a charm. fabulous! 🌈 🦄 🌟

awesome, I'll be doing the other packages then as follow-ups. Best to leave #1166 open until we are done.

[shilman]

@joscha @ndelangen @tmeasday I'd like to revert this change for now. It's a breaking change, so technically it would bump this release to 4.0.0 under semver, and I think we want to batch up breaking changes so that we aren't doing major releases all the time.

1. does this make sense to you guys?
2. (OR do you want to ditch semver and just release this is as 3.0.2, which is what it feels like?)
3. @joscha if we backed out this change, is it still possible for users to force typescript to use your DefinitelyTyped typings instead of these?

Thanks and sorry for the confusion!

UPDATED: going to release 3.1.0 instead even though it's not strict semver, and we can discuss this later as a group for how we want to do this in the future.

[joscha]

does this make sense to you guys?

makes sense, yes.

(OR do you want to ditch semver and just release this is as 3.0.2, which is what it feels like?)

hmm, not sure how annoyed I would be if I had to run one little yarn add @types/node @types/storybook__react after upgrading, I personally would probably be okay with it, but it is not a no-brainer update as a patch should be, so maybe 4.0 sounds right?

@joscha if we backed out this change, is it still possible for users to force typescript to use your DefinitelyTyped typings instead of these?

I don't think so, the typings definition in package.json takes precedence iirc, so it would shadow anything from @types/.... We'd need to keep the typings in definitelytyped updated alongside the ones in this repo for the time being and then remove the typings from this repo with the 4.0.0 release. Quite tedious and potentially confusing, but an easier upgrade path for users I suppose.

I haven't tried, but what if @types/storybook__react is made a dependency of @storybook/react for now and we reference the ./node_modules/@types/storybook__react/index.d.ts from the typings key in package.json - does that work? Because then that would be a seamless upgrade until in 4.0.0 we can remove the absolute dependency as a breaking change. The only penalty to pay in the meantime is the weight of the additional package for users that don't use TS, but they currently pay a similar penalty with the additional type definition anyway.

[shilman]

Sorry for the confusion @joscha . See my updated comment above. 😵