Skip to content
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

Please update the dependency for React-Native 0.43.x #893

Closed
shilman opened this issue Apr 15, 2017 · 13 comments
Closed

Please update the dependency for React-Native 0.43.x #893

shilman opened this issue Apr 15, 2017 · 13 comments

Comments

@shilman
Copy link
Member

shilman commented Apr 15, 2017

Issue by xareelee
Friday Apr 07, 2017 at 06:15 GMT
Originally opened as storybook-eol/storyshots#95


I upgraded React-Native from 0.42.0 to 0.43.2, and I encountered new test errors Invariant Violation: addComponentAsRefTo(...):

  ● Storyshots › [App] Root › entry

    Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. You might be adding a ref to a component that was not created inside a component's `render` method, or you have multiple copies of React loaded (details: https://fb.me/react-refs-must-have-owner).
      
      at invariant (node_modules/fbjs/lib/invariant.js:44:15)
      at Object.addComponentAsRefTo (node_modules/react-test-renderer/lib/ReactOwner.js:68:68)
      at attachRef (node_modules/react-test-renderer/lib/ReactRef.js:23:16)
      at Object.<anonymous>.ReactRef.attachRefs (node_modules/react-test-renderer/lib/ReactRef.js:42:5)
      at ReactCompositeComponentWrapper.attachRefs (node_modules/react-test-renderer/lib/ReactReconciler.js:23:12)
      at CallbackQueue.notifyAll (node_modules/react-test-renderer/lib/CallbackQueue.js:76:22)
      at ReactTestReconcileTransaction.close (node_modules/react-test-renderer/lib/ReactTestReconcileTransaction.js:36:26)
      at ReactTestReconcileTransaction.closeAll (node_modules/react-test-renderer/lib/Transaction.js:206:25)
      at ReactTestReconcileTransaction.perform (node_modules/react-test-renderer/lib/Transaction.js:153:16)
      at batchedMountComponentIntoNode (node_modules/react-test-renderer/lib/ReactTestMount.js:69:27)

I found that I have two version of react in my node_modules according to yarn.lock.

react@16.0.0-alpha.6:
  version "16.0.0-alpha.6"
  resolved "https://registry.yarnpkg.com/react/-/react-16.0.0-alpha.6.tgz#2ccb1afb4425ccc12f78a123a666f2e4c141adb9"
  dependencies:
    fbjs "^0.8.9"
    loose-envify "^1.1.0"
    object-assign "^4.1.0"

react@^15.4.1:
  version "15.4.2"
  resolved "https://registry.yarnpkg.com/react/-/react-15.4.2.tgz#41f7991b26185392ba9bae96c8889e7e018397ef"
  dependencies:
    fbjs "^0.8.4"
    loose-envify "^1.1.0"
    object-assign "^4.1.0"

And I found only storyshots depends on react@^15.4.1:

storyshots@^3.2.2:
  version "3.2.2"
  resolved "https://registry.yarnpkg.com/storyshots/-/storyshots-3.2.2.tgz#0e2027a86acc319f8100124224f0e3f09c29efc8"
  dependencies:
    babel-runtime "^6.20.0"
    react "^15.4.1"
    react-test-renderer "^15.3.1"
    read-pkg-up "^2.0.0"

Is the dependency necessary? Or could you update the dependency to latest react (react@16.0.0)?

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by ndelangen
Friday Apr 07, 2017 at 20:29 GMT


You're using an alpha version of react: react@16.0.0-alpha.6
'latest' react is '15.4.2': '2017-01-06T20:16:25.738Z',. Our version range should allow this version to be de-duplicated (and does according to your yarn.lock).

I don't feel we can add a react alpha version into our version range. What do you think about this @mnmtanish ?

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by xareelee
Saturday Apr 08, 2017 at 03:59 GMT


@ndelangen Thanks for your reply.

I'm using the latest React-Native (v0.43.x) which depends on react@16.0.0-alpha.6, and it leads the storyshots broken when jesting due to having multiple copies of React. We need the update for some fixes and features in RN 0.43.x.

Maybe we could modify the dependency of storyshots from ^15.4.1 to >=15.4.1 (not sure) or ^15.4.1 || >=16.0.0-alpha.6, and let the developers choose which version of React to be installed explicitly in package.json.

How about that?

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by ndelangen
Saturday Apr 08, 2017 at 09:40 GMT


If you edit the version range yourself and re-install does it work? If we're compatible with this version of React, I see no problem in adding it to our version range.

But if there's something we need to fix to support a alpha version, we can't just add it.

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by mnmtanish
Saturday Apr 08, 2017 at 10:56 GMT


@ndelangen yeah, we may end up making changes to storybook code to support alpha versions which may be unnecessary when the stable version comes out. @xareelee if it's okay, let's wait until an rc version comes out so it'll be stable enough.

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by xareelee
Saturday Apr 08, 2017 at 15:25 GMT


Got it. I'll try it by myself.

If you edit the version range yourself and re-install does it work?

How do I do that? Do you mean forking this repo and using it as a private module?

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by ndelangen
Sunday Apr 09, 2017 at 08:40 GMT


That's one way, you can also edit the installed package..

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by xareelee
Monday Apr 10, 2017 at 02:44 GMT


@ndelangen

  • I tried to edit the dependency in ./node_modules/storyshots/package.json and run yarn install, but it didn't work.
  • I tried to edit the dependency in yarn.lock and run yarn install, it seems OK. The tests are passed again.

Did I do right?

@xareelee
Copy link

xareelee commented May 17, 2017

@ndelangen

Is it possible changing the React dependency to peer dependency? The user using this lib should take the responsibility to install corresponding dependencies (React, in this case). I think this will avoiding the accidentally updating the yarn.lock file back to react 15.x in some npm/yarn operations. Am I right?

The React team claims that React 16.0.0-alpha is not harmful for React 15.x users, and ReactNative use React 16.0.0-alpha in the latest two release versions (0.43.x and 0.44.x).

This issue is really bothering me. :(

@ndelangen
Copy link
Member

ndelangen commented May 17, 2017

Your wish has already come true! It will be in the upcoming alpha release soon!
https://github.com/storybooks/storybook/blob/master/addons/storyshots/package.json#L35

All packages we release will/should now have react and react-dom as peerDependency.
prop-types is a regular dependency, because not everyone has migrated over or is using PropTypes.

And we also don't care about which version.
It's up to you to make sure works with your version of react.
You can see which version we develop with here: https://github.com/storybooks/storybook/blob/master/addons/storyshots/package.json#L21

@MoOx
Copy link

MoOx commented May 24, 2017

Where can we find storyshots in v3 alpha release? The link above is dead :(

@shilman
Copy link
Member Author

shilman commented May 24, 2017

@ndelangen
Copy link
Member

I fixed the links, sorry. storyshots got classified as an addon, and was moved in source. corrected now.

@ericwooley
Copy link
Contributor

ericwooley commented May 24, 2017

Get storybooks is great and all, but with all the deprecated repos, and different packages kadira/storybook, kadira/storyshots, storyshots, storybook/storybook etc...

When I am trying to upgrade it's really hard to figure out what goes with what other package.

With the release of the alpha versions, could you add docs for manual installation / upgrading? Also, is the alpha versions compatible with react 15 or 16 alpha only?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants