Fix/react version backwards compatibility #5148

ndelangen · 2019-01-04T15:48:16Z

What I did

Ensure usage of react installed by user inside preview
FIX compatibility issue with addon-info (it cannot use @storybook/components)

How to test

I changed the versions of package.json the cra example, and verified it would work, then changed it back.

See: https://5c2f898b5f9f740009255238--storybooks-cra.netlify.com/?selectedKind=Button&selectedStory=with%20new%20info&full=0&addons=1&stories=1&panelRight=1&addonPanel=storybook%2Factions%2Factions-panel

most changes are related to addon-info

codecov · 2019-01-04T16:59:47Z

Codecov Report

Merging #5148 into master will increase coverage by <.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #5148      +/-   ##
==========================================
+ Coverage   34.86%   34.87%   +<.01%     
==========================================
  Files         569      569              
  Lines        7036     7037       +1     
  Branches      940      938       -2     
==========================================
+ Hits         2453     2454       +1     
  Misses       4084     4084              
  Partials      499      499

Impacted Files	Coverage Δ
addons/info/src/components/markdown/pre/pre.js	`50% <ø> (-5.56%)`	⬇️
addons/info/src/components/Story.js	`88.88% <ø> (ø)`	⬆️
addons/info/src/components/markdown/htags.js	`69.23% <ø> (ø)`	⬆️
addons/info/src/components/markdown/text.js	`77.77% <ø> (ø)`	⬆️
...b/core/src/server/preview/iframe-webpack.config.js	`0% <0%> (ø)`	⬆️
...ons/info/src/components/markdown/pre/copyButton.js	`100% <100%> (ø)`	⬆️
addons/info/src/components/PropTable.js	`96.77% <100%> (+0.77%)`	⬆️
addons/info/src/components/types/Shape.js	`28.57% <33.33%> (+7.14%)`	⬆️

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 9a8eac6...146f0d6. Read the comment docs.

…der version of react

tmeasday

I don’t get it. Didn’t we already make this change in a different pr? (To iframe.webpack.js).

ndelangen · 2019-01-05T08:55:32Z

@tmeasday indeed, I'm confused as well, where did this change go we worked on..

Hypnosphi · 2019-01-05T12:34:32Z

addons/info/src/components/PropTable.js

 import PropVal from './PropVal';
 import PrettyPropType from './types/PrettyPropType';

+const Table = props => <table style={{}} {...props} />;


what's the point of adding an empty style object?

right nothing, doesn't output anything either though

Why not just use table then?

I don't feel strongly about this, but I like the look of jsx when it's just components, vs components & html mixed.

I don't think there are any pros or cons to speak of really.

One con is that it makes code less understandable at a first glance. It makes you think that Table does something additional when caomparing to table so you won't skip it e.g. when debugging

If you really want camelcase, you can do this:

const Table = 'table'

shilman · 2019-01-06T06:58:24Z

@ndelangen @tmeasday I'm confused about the status of these changes.

I checked out this branch and downgraded react/react-dom in examples/cra-kitchen-sink/package.json and ran storybook successfully
I checked out the next branch and did the same thing and ran yarn storybook and expected it to fail, but it succeeded.

Do we have a failing example in the repo that I can test to verify the fix?

Also, what's up with "part 1"? Are there more parts coming?

ndelangen · 2019-01-06T09:57:02Z

I checked out the next branch and did the same thing and ran yarn storybook and expected it to fail, but it succeeded.

You possibly forgot to regenerate the dll?

ndelangen · 2019-01-06T09:59:21Z

Also, what's up with "part 1"? Are there more parts coming?

just ignore, I was expecting to need to experiment a lot, since I thought @tmeasday and me already fixed this.

shilman · 2019-01-06T10:31:09Z

@ndelangen I did yarn bootstrap --reset --core and it regenerated the DLLs

shilman · 2019-01-07T09:14:40Z

I tried to test this out in the Wix example by creating build-packs and modifying their package.json as follows:

    "@storybook/addon-links": "file:/Users/shilman/projects/storybook/new/storybook/packs/storybook-addon-links.tgz",
    "@storybook/addon-options": "file:/Users/shilman/projects/storybook/new/storybook/packs/storybook-addon-options.tgz",
    "@storybook/react": "file:/Users/shilman/projects/storybook/new/storybook/packs/storybook-react.tgz",
    "@storybook/core": "file:/Users/shilman/projects/storybook/new/storybook/packs/storybook-core.tgz",
    "@storybook/ui": "file:/Users/shilman/projects/storybook/new/storybook/packs/storybook-ui.tgz",
    "@storybook/addons": "file:/Users/shilman/projects/storybook/new/storybook/packs/storybook-addons.tgz",
    "@storybook/components": "file:/Users/shilman/projects/storybook/new/storybook/packs/storybook-components.tgz",

I'm still getting the TypeError: Object(...) is not a function repro after doing this, though I'm not entirely sure whether I'm doing it properly. I also tested @latest and @next with same results.

…eact version for lib/core

because otherwise the wrong version is installed in the root

tmeasday · 2019-01-08T10:28:35Z

A couple of notes on this:

I think @ndelangen and my previous solution (not aliasing react[-dom] in this file) is better than using resolve-from and has the same effect. See Remove unnecessary dependencies, fix aliasing #5047.
I think the issue that that version (4.2.0-alpha.10) has with wix is that we didn't also change @storybook/ui to directly depend on react[-dom], an issue that is solved in this PR (in this commit f7c325b).
I'm not sure what's going on in this PR with addon-info, but I'll just note that wix are using addon-links which depends on emotion too. However they had already hard coded it to the 3.4 version and I suspect that version will continue to work with 4.1.

My advice would be to remove the first commit from this PR and rebase it onto #5047 but I don't have any bandwidth to look at it right now so if we just want to continue on that's fine.

shilman · 2019-01-09T04:05:59Z

@ndelangen I've released this as 4.1.5-react-debug.0 and confirmed that it fixes the issue with wix-style-react. I tried a simpler fix proposed by @tmeasday that just cherry-picks f7c325b onto master but I'm getting the following kinds of errors in createDlls (which don't occur on master and also don't occur in this PR). I suspect that there's some simple explanation, but unless I hear back soon I'm just going to merge and release this PR. I'll also take care of the addon-links issue that Tom mentioned before releasing.

ERROR in /Users/shilman/projects/storybook/new/storybook/node_modules/@storybook/react-komposer/dist/compose.js
Module not found: Error: Can't resolve '@storybook/react-stubber' in '/Users/shilman/projects/storybook/new/storybook/node_modules/@storybook/react-komposer/dist'
 @ /Users/shilman/projects/storybook/new/storybook/node_modules/@storybook/react-komposer/dist/compose.js 45:20-55
 @ /Users/shilman/projects/storybook/new/storybook/node_modules/@storybook/react-komposer/dist/index.js
 @ ./dist/compose.js
 @ ./dist/index.js
 @ dll storybook_ui

ERROR in ./dist/modules/ui/components/search_box.js
Module not found: Error: Can't resolve 'react-fuzzy' in '/Users/shilman/projects/storybook/new/storybook/lib/ui/dist/modules/ui/components'
 @ ./dist/modules/ui/components/search_box.js 36:41-63
 @ ./dist/modules/ui/containers/search_box.js
 @ ./dist/modules/ui/routes.js
 @ ./dist/modules/ui/index.js
 @ ./dist/index.js
 @ dll storybook_ui

[...more follow]

UPDATE: FWIW same error when I apply that commit to #5047

shilman

Approving this because I've verified that it works--not because I understand the solution or the alternatives. I've also fixed addon-links in an ugly way and have verified the fix.

shilman · 2019-01-09T06:39:23Z

@Hypnosphi The typescript CRA example Teamcity config has been merged and is now being applied to the master branch, which doesn't contain the TS CRA example. I'm ignoring the build error and merging, but if you have any suggestions on a better solution for this (cherry-pick the PR into master? OR ... ?) any ideas are appreciated. Thanks!

Hypnosphi · 2019-01-09T17:52:59Z

@shilman You just need to pass false as third argument in master branch to skip the actual build, see e.g. PREACT https://github.com/storybooks/storybook/blob/master/.teamcity/OpenSourceProjects_Storybook/buildTypes/StorybookApp.kt#L20

ndelangen added 3 commits January 4, 2019 16:14

FIX react -any version- compatibility

31f0f4a

FIX react < 16 compat for addon-info

a473b8e

TEMP showing that with an older version, things work as well

4d275e1

ndelangen added the react label Jan 4, 2019

ndelangen self-assigned this Jan 4, 2019

ndelangen requested a review from Hypnosphi January 4, 2019 15:48

ndelangen requested review from alterx, dangreenisrael, danielduan, igor-dv, theinterned, tmeasday, usulpro and z4o4z as code owners January 4, 2019 15:48

ndelangen added bug patch:yes Bugfix & documentation PR that need to be picked to main branch labels Jan 4, 2019

FIX snapshots

f1fe4d3

REVERT the package.json change to demo cra example working with an ol…

779855c

…der version of react

tmeasday reviewed Jan 4, 2019

View reviewed changes

Hypnosphi reviewed Jan 5, 2019

View reviewed changes

CLEANUP

f34a35e

ndelangen requested review from shilman and thomasbertet as code owners January 7, 2019 20:32

ndelangen added 2 commits January 7, 2019 21:33

ADD latest version of react to lib/ui (always) && REMOVE version of r…

f7c325b

…eact version for lib/core

ADD react & react-dom as depedencies to root

07770e1

because otherwise the wrong version is installed in the root

Emotion-free routed link

146f0d6

shilman changed the title ~~Fix/react versions part1~~ Fix/react version backwards compatibility Jan 9, 2019

shilman approved these changes Jan 9, 2019

View reviewed changes

shilman merged commit 297d979 into master Jan 9, 2019

shilman deleted the fix/react-versions-part1 branch January 9, 2019 06:39

shilman mentioned this pull request Jan 9, 2019

Merge react versions fix from master to next #5192

Merged

shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jan 9, 2019

shilman mentioned this pull request Jan 9, 2019

Storybook 4 not compatible with react 15 #4191

Closed

galefebvre-adeo mentioned this pull request Jan 10, 2019

@storybook/[email protected] breaks module resolving if react folder in user sources #5204

Closed

shilman mentioned this pull request Jan 17, 2019

[Addon Info] - upgrading from 4.0.12 to 4.1.6 missing styling for prop tables #5278

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/react version backwards compatibility #5148

Fix/react version backwards compatibility #5148

ndelangen commented Jan 4, 2019 •

edited

Loading

codecov bot commented Jan 4, 2019 •

edited

Loading

tmeasday left a comment

ndelangen commented Jan 5, 2019

Hypnosphi Jan 5, 2019

ndelangen Jan 5, 2019

Hypnosphi Jan 5, 2019

ndelangen Jan 6, 2019

Hypnosphi Jan 6, 2019 •

edited

Loading

shilman commented Jan 6, 2019 •

edited

Loading

ndelangen commented Jan 6, 2019

ndelangen commented Jan 6, 2019

shilman commented Jan 6, 2019

shilman commented Jan 7, 2019 •

edited

Loading

tmeasday commented Jan 8, 2019 •

edited

Loading

shilman commented Jan 9, 2019 •

edited

Loading

shilman left a comment

shilman commented Jan 9, 2019

Hypnosphi commented Jan 9, 2019 •

edited

Loading

Fix/react version backwards compatibility #5148

Fix/react version backwards compatibility #5148

Conversation

ndelangen commented Jan 4, 2019 • edited Loading

What I did

How to test

codecov bot commented Jan 4, 2019 • edited Loading

Codecov Report

tmeasday left a comment

Choose a reason for hiding this comment

ndelangen commented Jan 5, 2019

Hypnosphi Jan 5, 2019

Choose a reason for hiding this comment

ndelangen Jan 5, 2019

Choose a reason for hiding this comment

Hypnosphi Jan 5, 2019

Choose a reason for hiding this comment

ndelangen Jan 6, 2019

Choose a reason for hiding this comment

Hypnosphi Jan 6, 2019 • edited Loading

Choose a reason for hiding this comment

shilman commented Jan 6, 2019 • edited Loading

ndelangen commented Jan 6, 2019

ndelangen commented Jan 6, 2019

shilman commented Jan 6, 2019

shilman commented Jan 7, 2019 • edited Loading

tmeasday commented Jan 8, 2019 • edited Loading

shilman commented Jan 9, 2019 • edited Loading

shilman left a comment

Choose a reason for hiding this comment

shilman commented Jan 9, 2019

Hypnosphi commented Jan 9, 2019 • edited Loading

ndelangen commented Jan 4, 2019 •

edited

Loading

codecov bot commented Jan 4, 2019 •

edited

Loading

Hypnosphi Jan 6, 2019 •

edited

Loading

shilman commented Jan 6, 2019 •

edited

Loading

shilman commented Jan 7, 2019 •

edited

Loading

tmeasday commented Jan 8, 2019 •

edited

Loading

shilman commented Jan 9, 2019 •

edited

Loading

Hypnosphi commented Jan 9, 2019 •

edited

Loading