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

Use react-dev-utils@next #3852

Merged
merged 4 commits into from
Jul 20, 2018

Conversation

pelotom
Copy link
Contributor

@pelotom pelotom commented Jul 7, 2018

Recently create-react-app published a new version of react-dev-utils on the next tag, which I believe should obviate the need for the @storybook/react-dev-utils fork. This reverts #3312 and instead makes storybook depend on react-dev-utils@next.

@ndelangen
Copy link
Member

@pelotom
is the failing test related to the change?

#!/bin/bash -eo pipefail
yarn test --coverage --runInBand --reactnative
yarn coverage
yarn run v1.6.0
$ node ./scripts/test.js --coverage --runInBand --reactnative
 PASS  examples/react-native-vanilla/__tests__/index.android.js (8.317s)
 PASS  examples/react-native-vanilla/__tests__/index.ios.js
 FAIL  examples/react-native-vanilla/__tests__/storyshots.js
  ● Test suite failed to run

    TypeError: Cannot read property 'replace' of null
      
      at node_modules/hosted-git-info/index.js:61:63
          at Array.map (<anonymous>)
      at fromUrl (node_modules/hosted-git-info/index.js:45:39)
      at Function.Object.<anonymous>.module.exports.fromUrl (node_modules/hosted-git-info/index.js:32:18)
      at Object.<anonymous> (node_modules/normalize-package-data/lib/fixer.js:150:36)
          at Array.forEach (<anonymous>)
      at Object.<anonymous> (node_modules/normalize-package-data/lib/fixer.js:144:31)
          at Array.forEach (<anonymous>)
      at Object.fixDependencies (node_modules/normalize-package-data/lib/fixer.js:137:41)
      at node_modules/normalize-package-data/lib/normalize.js:32:38

@pelotom
Copy link
Contributor Author

pelotom commented Jul 7, 2018

@ndelangen I'm not really familiar with the dev workflow for this project... I ran

yarn test --core

and all tests pass, but when I try to run

yarn test --reactnative

I get

● Validation Error:

  Module <rootDir>/node_modules/react-native/jest/assetFileTransformer.js in the transform option was not found.

  Configuration Documentation:
  https://facebook.github.io/jest/docs/configuration.html

error Command failed with exit code 1.

even though <rootDir>/node_modules/react-native/jest/assetFileTransformer.js exists... any idea what I'm doing wrong?

@@ -29,7 +29,7 @@
"@storybook/channel-websocket": "4.0.0-alpha.12",
"@storybook/core": "4.0.0-alpha.12",
"@storybook/core-events": "4.0.0-alpha.12",
"@storybook/react-dev-utils": "^5.0.0",
"react-dev-utils": "next",
Copy link
Member

@danielduan danielduan Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use the version number instead of relying on the next tag. I think we're looking for react-dev-utils@^6.0.0-next.3e165448. the next version could easily be an alpha of 7.0.0-x in the future and cause unintended changes.

Copy link
Member

@danielduan danielduan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use actual version number

@codecov
Copy link

codecov bot commented Jul 18, 2018

Codecov Report

Merging #3852 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3852   +/-   ##
=======================================
  Coverage   41.56%   41.56%           
=======================================
  Files         455      455           
  Lines        5177     5177           
  Branches      899      899           
=======================================
  Hits         2152     2152           
  Misses       2485     2485           
  Partials      540      540
Impacted Files Coverage Δ
lib/core/src/server/config/webpack.config.js 0% <0%> (ø) ⬆️
...p/react-native/src/server/config/webpack.config.js 0% <0%> (ø) ⬆️
lib/core/src/server/config/webpack.config.prod.js 0% <0%> (ø) ⬆️

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 9211dcb...211ee17. Read the comment docs.

@ndelangen
Copy link
Member

@danielduan do your approve?

@ndelangen ndelangen merged commit 1d0bad6 into storybookjs:master Jul 20, 2018
@ndelangen
Copy link
Member

Thank you very much @pelotom!

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

Successfully merging this pull request may close these issues.

4 participants