-
Notifications
You must be signed in to change notification settings - Fork 23
Pass options to test renderer #98
Pass options to test renderer #98
Conversation
…dividual story level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is straight 🔥
const getTestRendererOptions = ( | ||
story, | ||
options = getRenderOptions(story) | ||
) => (options.createNodeMock) ? { createNodeMock: options.createNodeMock } : null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps plain JS functions would be better here for hoisting and/or readability reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think hoisting is an issue here as getTestRenderOptions
is defined before use in the exported testStorySnapshots
function.
stories/__test__/storyshots.test.js
Outdated
@@ -1,2 +1,3 @@ | |||
import initStoryshots from '../../src' | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could drop this whitespace change for a smaller diff :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very true. I'll fix that up
@@ -0,0 +1,36 @@ | |||
import React, { PropTypes } from 'react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this project use React v15.5? If so it should bring in prop-types
separately [and if not ignore this]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the storybooks/storyshots repo is running at "react": "^15.4.1"
. I didn't want to update dependencies and stuff here as the repo is deprecated ...
@theinterned could we avoid complex changes, and just leverage a custom // in the story, wrap with a "box"
storiesOf(...)
.add((
<RenderWithOptions createNodeMock={...}>
<ComponentWithRef .../>
</RenderWithOptions>
));
// when setting up storyshots, provide a custom test function
initStoryshots({
test(renderedStory) {
let options = {};
// XXX untested psuedo-code but you get the idea
if (renderedStory.displayType === 'RenderWithOptions') {
renderedStory = renderedStory.children;
options = renderedStory.props;
}
const tree = renderer.create(renderedStory, options).toJSON();
expect(tree).toMatchSnapshot()
}
}); Then someone could make a simple package that exports the Proposal for |
@tmeasday I like where you are going with the custom Maybe lifecycle methods like |
@tmeasday my other comment is that being able to pass options to the |
Okay, I've made a proposal for a generic solution to adding stories: storybookjs/storybook#993 |
Issue:
When running storyshots with a story for a component that uses refs, it will throw a null reference error (see facebook/react#7371)
What I did
This PR adds the ability to pass options to a story's render function.
Specifically, a
createNodeMock
function can be passed throughstory.render.options.createNodeMock
toreact-test-renderer
. This allows refs to be mocked as per https://facebook.github.io/react/blog/2016/11/16/react-v15.4.0.html#mocking-refs-for-snapshot-testingHow to test
Run yarn test -
ComponentWithRef.stories.js
should use thecreateNodeMock
function defined inComponentWithRef.stories.js:5
to mock its ref and not fail with a null reference error. The story forComponentWithRef
attempts to get thescrollWidth
of a div when the component mounts.Thanks
With thanks to @jrdrg for the inspiration here: storybookjs/storybook#896
Note
I am submitting this to the deprecated StoryShots repo as tests don't run in the StoryShots package in the Storybook mono repo. I hope to re-submit this PR to the mono repo when 3.0 is released (or as soon as test running is fixed in the StoryShots package)