Shallow rendering broke when upgrading to Storybook 5

[christopher-francisco]

Describe the bug
The shallow rendered serialized snapshots broke after I upgraded from 3 to 4 and 4 to 5.

It used to render the inside of my component:

<article>
  <h1>Foo</h1>
</article>

But now it's rendering the ...outside(?) of it

<MyComponent
  title="Foo"
/>

To Reproduce
Steps to reproduce the behavior:

1. Update the following packages

-"@storybook/addon-storyshots": "^3.4.7",
+"@storybook/addon-storyshots": "^5.1.3",
-"@storybook/react": "^3.4.6",
+"@storybook/react": "^5.1.3",

Expected behavior
My snapshots should not break, or at least should not render completely different output

Code snippets

import { shallow } from 'enzyme';
import initStoryshots from '@storybook/addon-storyshots';

initStoryshots({
  storyKindRegex: /^MyComponent$/,
  renderer: shallow,
});

System:

• OS: MacOS 10.13.6
• Device: Macbook Pro 2018
• Framework: create-react-app, jest
• Addons: storyshots
• Version: 5.1.3

Update

Using this works

 initStoryshots({
-  renderer: shallow,
+  renderer: (...args) => shallow(...args).dive(),
 })

Update 2

But it doesn't work if you're unwrapping your component from an HOC, like I am, sadly:

const intl = getItFromSomeOtherModule() // react-intl stuff
const unwrap = node => {
  if (node.type.name === 'InjectIntl') {
    const unwrappedType = node.type.WrappedComponent;
    node = React.createElement(unwrappedType, { ...node.props, intl });
  }

  return node;
}

// storyshot test file
renderer: (node, options) => shallow(unwrap(node), options).dive(); // doesn't work.

Update 3

Seems like the problem is that the node argument received by the renderer function (in the signature (node, options => shallow(node, options)) is a <ReactDecorator />, rather than a <MyComponent />.

For me, particulary, it breaks the suite as most of my components are wrapped by HoCs that I don't want to shallow render (ergo the .WrappedComponent pattern to unwrap). I don't have a workaround.

Update 4

Fixed by commenting out the console plugin.
Forget everything above, just comment out this plugging and that'd be it.

// import '@storybook/addon-console';
// import { withConsole } from '@storybook/addon-console';
// addDecorator((storyFn, context) => withConsole()(storyFn)(context));

For a long term solution I'll just be making use of configPath property on storyshots to pass a config file that does not include this decorator

[shilman]

@chris-fran Thanks for the bug report and also for the workaround! No sure what's going on here, but maybe somebody more familiar with storyshots can chime in. At least we should add your workaround to MIGRATION.md. cc @igor-dv @tmeasday

Also, do you happen to know if this worked in SB4?

[christopher-francisco]

@shilman I don't, I just followed the migration guide to 4 but it got me to 5 instantly (painlessly so that was cool).

I am wondering whether the problem I'm seeing is the same as this one #6880

[christopher-francisco]

@shilman the workaround doesn't always work. Doesn't work for HOC wrapped components that you try to unwrap with the component.WrappedComponent pattern

[tmeasday]

@shilman I don't know we've ever really resolved this question of storyshots/shallow rendering/decorators.

In SB5 we added a getOriginal function to the story context that you could use to drop decorators.

However I think this is an unsatisfactory solution because in many cases the decorators are needed to render the story properly (as opposed to used for layout or utils like the console addon) -- decorators that do things like supply context to components.

Once upon a time we proposed classify decorators into various kinds, although that seems pretty heavy too. I'm not sure what the best way forward in the short or long term is.

[christopher-francisco]

@tmeasday I'm a huge shallow rendering advocator; it is the closest, if not, to a unit test for a serialized component. From my point of view, when shallow rendering, the decorators are not wanted. I think a more satisfactory solution would be to opt-in/out of receiving or not the decorators.

I'm sure they're needed for rendering the complete story, and even for image snapshot tests; but for shallow rendering, I'd like for a way not to get the decorators, so that I can pass whatever the decorator is passing down myself:

// MyComponent.js
import { injectIntl } from 'react-intl'

let MyComponent = ({ intl }) => <div>{intl.formatMessage({ id: 'hello.world' })}</div>
let MyComponent = injectIntl(MyComponent)

// MyComponent.test.js
import { intl } from '../test-utils/intl'

const unwrap = node => {
  unwrappedType = node.type.WrappedComponent
  return React.createElement(unwrappedType, { ...node.props, intl })
}

// by unwrapping with the function above, I can pass a stub intl object
// and don't need `<IntlProvider>` in the hierarchy, and don't need `.dive()`

shallow(unwrap(MyComponent), { intl })

[tmeasday]

@chris-fran perhaps you could try using getOriginal and writing a custom shallow renderer storyshots renderer? If it works well perhaps you could report back here (or on a new ticket perhaps).

I'm sure they're needed for rendering the complete story, and even for image snapshot tests; but for shallow rendering, I'd like for a way not to get the decorators, so that I can pass whatever the decorator is passing down myself:

I'm wondering about this.. what does the test-utils version of intl do? Are you doing things like checking certain things are called on it or something? Or is it just faster than the real one?

I guess I am not quite understanding why if you have some sensible mock you'd not use that in stories also.

[stale]

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]

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!

[alber70g]

From my point of view, when shallow rendering, the decorators are not wanted. I think a more satisfactory solution would be to opt-in/out of receiving or not the decorators.

Is this something that can be done? Even in the decorator knowing whether we're in a snapshot test or live preview, could be of help.

const withThemeProvider = (Story, context) => {
  if (context.type === 'snapshot') {
    return <Story />
  }
}

[tmeasday]

That's definitely one option @alber70g -- in fact you could even write a "higher order decorator" that made any given decorator behave like that.

We have talked about this issue in various other contexts and it still seems like something that we don't have a great answer to yet.