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

Storyshots does not render JSX in addDecorator #9771

Closed
david-golightly-leapyear opened this issue Feb 6, 2020 · 15 comments
Closed

Storyshots does not render JSX in addDecorator #9771

david-golightly-leapyear opened this issue Feb 6, 2020 · 15 comments
Assignees
Projects
Milestone

Comments

@david-golightly-leapyear
Copy link

david-golightly-leapyear commented Feb 6, 2020

Describe the bug
addDecorator from storybook/react does not appear to be called in storyshots, even though it's called in Storybook itself.

[EDIT] This appears to be the case only in .jsx and .tsx files.

To Reproduce
Minimal repro can be found at this repository:
https://github.com/LeapYear/storybook-storyshots-repro

Expected behavior
addDecorator should be called with storyshots

Possibly related issues
#3246

@shilman
Copy link
Member

shilman commented Feb 6, 2020

@david-golightly-leapyear It looks to me like it actually recommends using main.js stories as the most convenient way to load stories, and documents configure as a less convenient alternative?

@david-golightly-leapyear
Copy link
Author

@shilman That’s correct. The linked repo uses the main.js recommendation.

@shilman
Copy link
Member

shilman commented Feb 6, 2020

My question was about the inconsistency comment. It doesn't seem inconsistent to me, but maybe I'm missing something? I want to fix it if it's inconsistent 😄

@david-golightly-leapyear
Copy link
Author

The resolution to #3246 recommends ensuring addDecorator is called before configure, but it’s not clear that migrating from configure to stories in main.js preserves the load order behavior necessary for decorators to be set up prior to story loading.

@shilman
Copy link
Member

shilman commented Feb 6, 2020

Gotcha. Thanks for clarifying! cc @ndelangen

@ndelangen ndelangen self-assigned this Feb 6, 2020
@ndelangen ndelangen added this to the 5.3.x milestone Feb 6, 2020
@iansan5653
Copy link

Confirmed - all of our stories rely on a theme context being present, but since this is provided by a decorator, we can't use storyshots at all.

@ndelangen
Copy link
Member

We're going to have to inspect the load order of preview.js in storyshots.

In the repro I see there's a preview.TSX. This might be the problem.
AFAK there's no support for TS in preview.js file.

@iansan5653
Copy link

In the meantime, is there any other way to add a decorator? I'm using the following to get styled-components to work; can I insert the decorator somewhere in this function?

addSerializer(styleSheetSerializer);

initStoryshots({
  test: ({story, context}) => {
    const snapshotFilename = path.join(
      `__snapshots__/${context.kind}.storysnaps`
    );

    const storyElement = story.render(context);
    const tree = renderer.create(storyElement).toJSON();
    expect(tree).toMatchSpecificSnapshot(snapshotFilename);
  }
});

@shilman
Copy link
Member

shilman commented Feb 7, 2020

@ndelangen pretty sure there's support for preview.tsx? #9775 cc @igor-dv

@david-golightly-leapyear
Copy link
Author

@ndelangen @shilman I can confirm that changing preview.tsx -> preview.jsx does not change any behavior here. However, if I rename the file to exactly preview.js, it does invoke the file & decorators in the right order. However, this means that preview.js cannot have any ES2015+ features, such as module imports or JSX. I had to change the file contents to the following to get it to work:

const { addDecorator } = require('@storybook/react')
const React = require('react')

addDecorator((content) => (
  React.createElement('div', { children: ['Preview Decorator', content()]})
))

This is sort of a workaround, but represents a significant behavioral regression from 5.2. Especially more complex React layouts will be tedious to implement in this way.

@shilman shilman added this to 5.3 bugs in Hotlist Feb 10, 2020
@shilman
Copy link
Member

shilman commented Feb 10, 2020

Looks like there are a couple bugs here @ndelangen:

const previewTS = path.join(configDir, 'preview.ts');

Can we just add support for [.tsx,.ts,.jsx,.js] across the board, in a simple find call?

@ndelangen
Copy link
Member

@david-golightly-leapyear would you happen to have some time to fix this issue?

@shilman has pointed out the correct place for the fix.

🙇

@david-golightly-leapyear
Copy link
Author

@ndelangen @shilman Submitted #9834 for your review.

@david-golightly-leapyear david-golightly-leapyear changed the title Storyshots does not render addDecorator Storyshots does not render JSX in addDecorator Feb 13, 2020
@shilman
Copy link
Member

shilman commented Feb 15, 2020

Ermahgerd!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-alpha.13 containing PR #9834 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Feb 15, 2020
@shilman
Copy link
Member

shilman commented Feb 25, 2020

Yowza!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.14 containing PR #9834 that references this issue. Upgrade today to try it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Hotlist
5.3 bugs
Development

No branches or pull requests

4 participants