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

Add flowtype example again #1623

Merged
merged 1 commit into from
Aug 18, 2017
Merged

Conversation

jribeiro
Copy link
Contributor

@jribeiro jribeiro commented Aug 8, 2017

Issue:

What I did

Added flowtype example again

How to test

Run cra-kitchen-sink example

@jribeiro
Copy link
Contributor Author

jribeiro commented Aug 8, 2017

Please see the attached image proving the example using flowtype is working. No additional libraries installed.
screen shot 2017-08-08 at 20 52 52

$ yarn global list
yarn global v0.27.5
warning package.json: No license field
warning No license field
info "[email protected]" has binaries:
   - babel-doctor
   - babel
   - babel-node
   - babel-external-helpers

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.

Everything else looks good to me.

@@ -0,0 +1,24 @@
// @flow
import React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

I don't think our local .eslintrc has rules for flow so it's probably a good idea to disable it for this file. /* eslint-disable */

Copy link
Member

@Hypnosphi Hypnosphi Aug 8, 2017

Choose a reason for hiding this comment

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

or, even better, add a local .eslintrc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Let me sort that out

Copy link
Member

Choose a reason for hiding this comment

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

seems a bit much to add rules for flow just to support an example file but either way, I'll take it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just out of curiosity, how do you get the linting error? I think following this setup should be enough btw:
https://github.com/gajus/eslint-plugin-flowtype#shareable-configurations

Copy link
Member

Choose a reason for hiding this comment

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

Had it in Atom with the linter plugin that reads from the nearest .eslintrc.

@ndelangen
Copy link
Member

snapshot needs an update:

FAIL  examples/cra-kitchen-sink/src/storyshots.test.js
  ● Console

    console.info node_modules/@storybook/react/dist/server/babel_config.js:73
      => Loading custom .babelrc
    console.error node_modules/fbjs/lib/warning.js:35
      Warning: DocgenButton: type specification of prop `style` is invalid; the type checker function must return `null` or an `Error` but returned a function. You may have forgotten to pass an argument to the type checker creator (arrayOf, instanceOf, objectOf, oneOf, oneOfType, and shape all require an argument).

  ● Storyshots › AddonInfo.DocgenButton › DocgenButton

    expect(value).toMatchSnapshot()
    
    New snapshot was not written. The update flag must be explicitly passed to write a new snapshot.
    
    This is likely because this test is run in a continuous integration (CI) environment in which snapshots are not written by default.
      
      at Object.test (node_modules/@storybook/addon-storyshots/dist/test-bodies.js:27:18)
      at Object.<anonymous> (node_modules/@storybook/addon-storyshots/dist/index.js:155:25)
          at Promise (<anonymous>)
      at Promise.resolve.then.el (node_modules/p-map/index.js:42:16)
          at <anonymous>

  ● Storyshots › AddonInfo.FlowTypeButton › FlowTypeButton

    expect(value).toMatchSnapshot()
    
    New snapshot was not written. The update flag must be explicitly passed to write a new snapshot.
    
    This is likely because this test is run in a continuous integration (CI) environment in which snapshots are not written by default.
      
      at Object.test (node_modules/@storybook/addon-storyshots/dist/test-bodies.js:27:18)
      at Object.<anonymous> (node_modules/@storybook/addon-storyshots/dist/index.js:155:25)
          at Promise (<anonymous>)
      at Promise.resolve.then.el (node_modules/p-map/index.js:42:16)
          at <anonymous>

@ndelangen
Copy link
Member

@danielduan if https://github.com/storybooks/storybook/tree/info-docgen is your branch, it's up to you to merge 👍

@danielduan danielduan merged commit a09e3b5 into storybookjs:info-docgen Aug 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants