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

[SIGNIFICANT] Remove redundant render in react #2503

Merged
merged 7 commits into from
Dec 21, 2017
Merged
18 changes: 10 additions & 8 deletions app/react/src/client/preview/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,18 @@ export function renderMain(data, storyStore) {
// renderMain() gets executed after each action. Actions will cause the whole
// story to re-render without this check.
// https://github.com/storybooks/react-storybook/issues/116
if (selectedKind !== previousKind || previousStory !== selectedStory) {
// We need to unmount the existing set of components in the DOM node.
// Otherwise, React may not recrease instances for every story run.
// This could leads to issues like below:
// https://github.com/storybooks/react-storybook/issues/81
previousKind = selectedKind;
previousStory = selectedStory;
ReactDOM.unmountComponentAtNode(rootEl);
if (selectedKind === previousKind || previousStory === selectedStory) {
Copy link
Member

Choose a reason for hiding this comment

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

&&?

Copy link
Member Author

Choose a reason for hiding this comment

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

😊

return null;
}

// We need to unmount the existing set of components in the DOM node.
// Otherwise, React may not recrease instances for every story run.
// This could leads to issues like below:
// https://github.com/storybooks/react-storybook/issues/81
previousKind = selectedKind;
previousStory = selectedStory;
ReactDOM.unmountComponentAtNode(rootEl);

const context = {
kind: selectedKind,
story: selectedStory,
Expand Down
39 changes: 39 additions & 0 deletions examples/cra-kitchen-sink/src/components/LifecycleLogger.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React, { Component } from 'react';

function log(name) {
console.log(`LifecycleLogger: ${name}`);
Copy link
Member

Choose a reason for hiding this comment

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

Feels like a linter warning ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think I should use addon-actions for this instead?

Copy link
Member

Choose a reason for hiding this comment

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

You can just ignore the warning =)

Copy link
Member

Choose a reason for hiding this comment

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

You should use @storybook/client-logger

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

}

// A component that logs its lifecycle so we can check that things happen
// the right number of times (i.e. we are using React properly)
export default class LifecycleLogger extends Component {
constructor() {
super();
log('contructor');
}
componentWillMount() {
log('componentWillMount');
}
componentDidMount() {
log('componentDidMount');
}
componentWillReceiveProps() {
log('componentWillReceiveProps');
}
componentWillUpdate() {
log('componentWillUpdate');
}
componentDidUpdate() {
log('componentDidUpdate');
}
componentWillUnmount() {
log('componentWillUnmount');
}
componentDidCatch() {
log('componentDidCatch');
}
render() {
log('render');
return <div>Lifecycle methods are logged to the console</div>;
}
}
3 changes: 3 additions & 0 deletions examples/cra-kitchen-sink/src/stories/index.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Button } from '@storybook/react/demo';
import App from '../App';
import Logger from './Logger';
import Container from './Container';
import LifecycleLogger from '../components/LifecycleLogger';

const EVENTS = {
TEST_EVENT_1: 'test-event-1',
Expand Down Expand Up @@ -102,6 +103,8 @@ storiesOf('Some really long story kind description', module)
.addDecorator(centered)
.add('with text', () => <Button onClick={action('clicked')}>Hello Button</Button>);

storiesOf('Lifecycle', module).add('logging', () => <LifecycleLogger />);

storiesOf('WithEvents', module)
.addDecorator(getStory => (
<WithEvents
Expand Down