storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.46k stars 9.28k forks source link

[Addon-storyshots] Hooks in story break tests #8177

Closed shmidt-i closed 4 years ago

shmidt-i commented 5 years ago

Describe the bug Recently the feature of using hooks directly inside the stories landed. Unfortunately, it is still an error when using addon-storyshots. Wrapping in component helps

To Reproduce Steps to reproduce the behavior:

storyOf('Test', module).add('error', () => {
  React.useEffect(() => {}, []);
  return null;
})

Expected behavior Storyshot with null in it

Screenshots

Invariant Violation: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one 
of the following reasons:
    1. You might have mismatching versions of React and the renderer (such as React DOM)
    2. You might be breaking the Rules of Hooks
    3. You might have more than one copy of React in the same app
    See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.

      165 |   })
      166 |   .add('error', () => {
    > 167 |     React.useEffect(() => {}, []);
          |           ^
      168 |     return null;
      169 |   })

System:

Environment Info:

System: OS: macOS High Sierra 10.13.6 CPU: (8) x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz Binaries: Node: 10.13.0 Yarn: 1.17.3 npm: 6.5.0 Browsers: Chrome: 77.0.3865.90 Firefox: 68.0.1 Safari: 13.0 npmPackages: @storybook/addon-a11y: 5.2.1 => 5.2.1 @storybook/addon-actions: 5.2.1 => 5.2.1 @storybook/addon-knobs: 5.2.1 => 5.2.1 @storybook/addon-links: 5.2.1 => 5.2.1 @storybook/addon-storyshots: 5.2.1 => 5.2.1 @storybook/addon-storysource: 5.2.1 => 5.2.1 @storybook/addons: 5.2.1 => 5.2.1 @storybook/react: 5.2.1 => 5.2.1

Additional context Add any other context about the problem here.

uyarn commented 5 years ago

Hooks can only be called inside of the body of a function component. Try to use CSF way to write your stories

shmidt-i commented 5 years ago

CSF maybe will solve the problem, although it's unlikely we are going this way with our codebase.

But, the original question is - according to the https://github.com/storybookjs/storybook/pull/7571 the hooks should work directly in the story. And they do. Until you try to run addon-storyshots, which is broken :)

Upd: looked more in the PR and yes, it seems it was oriented towards CSF usage, but for some reason the code above works when you run storybook.

shilman commented 5 years ago

@shmidt-i i'm curious why you're unlikely to adopt CSF in your codebase

shmidt-i commented 5 years ago

We are using storybook more like a toolset for TDD, that's why all our stories resemble tests descriptions

describe('counter interactivity', () => {
  it('should increment when enabled[special case] (EXPERIMENTAL)', () => {

We don't want (at least for now) to do

export default {title: 'counter interactivity'}
export const shouldIncrementWhenEnabled_specialCase__EXPERIMENTAL = ...
shmidt-i commented 5 years ago

And I totally get the benefit of switching towards CSF in a long run, and believe me we've waited a lot for docs and MDX to happen, but that switch cannot be done in one moment and we want to use hooks as first-class citizens already :)

Also, great job on recent changes!!!

shilman commented 5 years ago

cool, we should support storiesOf for awhile so it shouldn't be a problem for you to stick with that. just wanted to know if there was something wrong with CSF specifically that was preventing you from upgrading, and it sounds like there's not.

hopefully we can fix this storyshots problem soon and get you unblocked with hooks!

tmikeschu commented 5 years ago

Confirming this issue:

    "@storybook/addon-actions": "^5.2.1",
    "@storybook/addon-links": "^5.2.1",
    "@storybook/addon-storyshots": "^5.2.1",
    "@storybook/addons": "^5.2.1",
    "@storybook/react": "^5.2.1",
    "babel-plugin-require-context-hook": "^1.0.0",
    "jest": "^24.9.0",
    "react": "^16.9.0",
    node: 10.15.3

Using this as a workaround:

const StoryWithHook = () => {
  const [value, setValue] = useState(0);
  return <div onClick={() => setValue(value + 1)}>{value}</div>;
};

export const storyWithHook = () => <StoryWithHook />;
stale[bot] commented 5 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

qooban commented 5 years ago

Workaround suggested by @tmikeschu works fine, but that way you lose the ability to show the code on the story page: image

B3Kay commented 4 years ago

I also got this problem. I want to render a story with hooks and also be able to show the code example. Not just using the workaround like @tmikeschu

qooban commented 4 years ago

@B3Kay I just checked the most recent v5.3.0-alpha.34 and it looks like it works fine in that version.

shilman commented 4 years ago

@B3Kay if you can verify this works in the alpha, I'd love to close this issue 😄

qooban commented 4 years ago

@shilman @B3Kay Sorry, my bad, I just checked if the docs are presented properly, but not checked if the storyshots are working fine. Unfortunatelly - this issue is still valid.

sdrissi commented 4 years ago

Hey there! Same issue for me with React hooks in CSF stories...Any idea when a fix will be available (I would gladly contribute to fix the issue but I'm not familiar with your codebase).

brandon-leapyear commented 4 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


Hi, coming across this issue using CSF. Am currently using this workaround:

+// https://github.com/storybookjs/storybook/issues/8177
+const storyshotsHack = (StoryFn: React.FC) => () => <StoryFn />

-export const basic = () => {
+export const basic = storyshotsHack(() => {
   const [isToggled, setIsToggled] = useState(false)
   return (
     <>
       <Text>Normal Toggle</Text>
       <Toggle value={isToggled} onClick={() => setIsToggled(!isToggled)} />
     </>
   )
-}
+})
sdrissi commented 4 years ago

Thanks @brandon-leapyear for your solution. I've tried it and it's a better alternative than the one suggested by @tmikeschu since it keeps the source code. @B3Kay this should do the trick

stale[bot] commented 4 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

chrisfosterelli commented 4 years ago

You can also wrap the story in React.createElement as the PR mentioned above does. It seems for some reason the PR applied to the stories in storybook but not for storyshots.

shilman commented 4 years ago

We could probably apply that here:

https://github.com/storybookjs/storybook/blob/next/addons/storyshots/storyshots-core/src/frameworks/react/renderTree.ts#L5

https://github.com/storybookjs/storybook/blob/next/addons/storyshots/storyshots-core/src/frameworks/react/renderShallowTree.ts#L5

Anybody want to give that a try?

tmikeschu commented 4 years ago

@shilman I can take a look tomorrow! I haven't contributed yet and I'd like to 😄

tmikeschu commented 4 years ago

https://github.com/storybookjs/storybook/pull/9187

tmikeschu commented 4 years ago

It's interesting that @brandon-leapyear 's solution is like a lazy identity function. Just food for thought!

stale[bot] commented 4 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

stale[bot] commented 4 years ago

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

Bjodol commented 4 years ago

Hey! Came over this issue and found a solution that simplify the hack. Add to your .storybook/preview.js:

// .storybook/preview.js
import { addDecorator } from '@storybook/react';
import React from 'react';

addDecorator(StoryFn => {
    return (
            <StoryFn />
    );
});
timwuhaotian commented 4 years ago

Confirming this issue:

    "@storybook/addon-actions": "^5.2.1",
    "@storybook/addon-links": "^5.2.1",
    "@storybook/addon-storyshots": "^5.2.1",
    "@storybook/addons": "^5.2.1",
    "@storybook/react": "^5.2.1",
    "babel-plugin-require-context-hook": "^1.0.0",
    "jest": "^24.9.0",
    "react": "^16.9.0",
    node: 10.15.3

Using this as a workaround:

const StoryWithHook = () => {
  const [value, setValue] = useState(0);
  return <div onClick={() => setValue(value + 1)}>{value}</div>;
};

export const storyWithHook = () => <StoryWithHook />;

It works for me, I am using jest and storyshots, initially it throws the Invalid hook call error, I checked the React duplicate issue and all other suggestions, but no luck.

until I find this solution!

qdozaq commented 4 years ago

@Bjodol 's solution is perfect and can actually be simplified even more!

// .storybook/preview.js
import { addDecorator } from '@storybook/react';
import {createElement} from 'react';

addDecorator(createElement);
vdh commented 4 years ago

These workarounds still break the snapshots though? AFAIK any shallow rendering is wrecked by this hack.

varundey commented 4 years ago

Facing same issue with react, react-dom and react-test-renderer v16.13.1

brandon-leapyear commented 4 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


FYI storybook works with hooks now: https://github.com/storybookjs/storybook/pull/7571

it would be great to get the same support with storyshots

shilman commented 4 years ago

Released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.43 which contains #10529 and may address this

michaelbayday commented 4 years ago

I'm still getting this issue. Does anyone have any suggestions?

main.js

// We are using the beta version of storybook 6.0 which address typescript issues with documentation.
// Addons and the library should be updated to the next major release.
const path = require('path');

module.exports = {
  stories: ['../src/**/*.stories.tsx'],
  addons: [
    '@storybook/addon-actions',
    '@storybook/addon-knobs',
    '@storybook/addon-a11y',
    '@storybook/addon-viewport',
    '@storybook/addon-docs',
    '@storybook/addon-links',
  ],
  webpackFinal: (config) => {
    config.resolve.alias = {
      ...config.resolve.alias,
      assets: path.resolve(__dirname, '../src/assets/'),
    };
    return config;
  },
};

manager.js

import { addons } from '@storybook/addons';
import theme from './theme';

addons.setConfig({
  theme,
});

preview.js

import React from 'react';
import { createGlobalStyle } from 'styled-components';
import { addDecorator } from '@storybook/react';
import { addParameters } from '@storybook/client-api';
import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport';

const GlobalStyle = createGlobalStyle`
  html {
    font-size: 62.5%;
    box-sizing: border-box;
  }
  *, *:before, *:after {
    box-sizing: inherit;
  }

  html, body {
    margin: 0;
    padding: 0;
    height: 100%;
    color: black;
  }
  * {
    font-family:  'HelveticaNeue', 'Helvetica Neue', Helvetica, Arial, sans-serif;
  }
  #root {
    font-size: 1.6rem;
  }
`;

addDecorator((storyFn) => (
  <>
    <GlobalStyle />
    {storyFn()}
  </>
));

addParameters({
  viewport: {
    viewports: INITIAL_VIEWPORTS,
  },
});

storyshots.test.js

import initStoryshots, { Stories2SnapsConverter } from '@storybook/addon-storyshots';
import renderer from 'react-test-renderer';
import { styleSheetSerializer } from 'jest-styled-components/serializer';
import { addSerializer } from 'jest-specific-snapshot';

addSerializer(styleSheetSerializer);

initStoryshots({
  test: ({ story, context }) => {
    const converter = new Stories2SnapsConverter();
    const snapshotFilename = converter.getSnapshotFileName(context);
    const storyElement = story.render(context);
    const tree = renderer.create(storyElement).toJSON();
    expect(tree).toMatchSpecificSnapshot(snapshotFilename);
  },
});

jest.config.js

module.exports = {
  preset: 'ts-jest/presets/js-with-ts',
  roots: ['src'],
  setupFilesAfterEnv: ['./jest.setup.js'],
  moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json'],
  transformIgnorePatterns: ['node_modules/(?!@storybook/*)'],
  testMatch: ['**/*.test.(js)'],
  moduleNameMapper: {
    // Mocks out all these file formats when tests are run.
    '\\.(jpg|ico|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
      '<rootDir>/__mocks__/fileMock.js',
  },
  globals: {
    'ts-jest': {
      tsConfig: 'tsconfig.test.json',
    },
  },
  collectCoverageFrom: ['src/**/*.tsx'],
};

jest.setup.js

import '@testing-library/jest-dom';

The serialization of the styled components is working, but just running into this last hiccup.

I'm running storybook v6.0.0-beta.39