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

build-storybook differences? #551

Closed
leebenson opened this issue Oct 18, 2016 · 8 comments
Closed

build-storybook differences? #551

leebenson opened this issue Oct 18, 2016 · 8 comments

Comments

@leebenson
Copy link

leebenson commented Oct 18, 2016

I'm trying to figure out a weird edge case I'm having, using mobx for store management inside stories.

Take this (highly convoluted) story:

import React from 'react';
import { observable } from 'mobx';
import { observer } from 'mobx-react';

// Create a new store
const store = observable({
  toggle: true,
});

// Toggle the value inside the store
function toggle() {
  store.toggle = !store.toggle;
}

// Test component -- uses the store above, but NOT a direct observer
const Headline = props => (
  <h1>{props.store.toggle.toString()}</h1>
);

Headline.propTypes = {
  store: React.PropTypes.shape({
    toggle: React.PropTypes.bool.isRequired,
  }),
};

// Generator func to return x number of <Headline> components, fed the store
function* getComponents(count) {
  for (let i = 1; i <= count; i++) {
    yield (
      <Headline key={i} store={store} />
    );
  }
}

// Wrap THIS one in an observer, and go down the chain
const Toggle = observer(() => (
  <div>
    {[...getComponents(5)]}
    <button
      onClick={toggle}>Toggle</button>
  </div>
));

// Export as an instantiated JSX comp.
export default <Toggle />;

When running in dev (npm run dev), clicking 'toggle' works fine...

works

However, when running in production (npm run build-storybook and starting a local server), toggle suddenly stops working...

not-working

My gut reaction is that it has something to do with transpilation differences. Stores built using MobX's observer function work differently depending on whether there's a prototype chain (if there is, properties on the object are observable, but not the parent object itself). But that doesn't seem to be happening because in both cases, there's no prototype chain, and in other examples where the parent store is not fed in but used directly, it works as expected.

It seems the observer chain is 'lost' somehow when the generator passes it to it.

I know it's a totally weird edge case, but are there any known differences between how code is rendered in dev vs. bundling?

@mweststrate
Copy link

Would have to take a closer look, but my gut feeling is that it might relate to this: mobxjs/mobx-react#56. Could you remove the propTypes and check whether the dev setup is not "accidentally" working correctly as a result of propTypes?

@mweststrate
Copy link

Not that MobX-react only supports direct observers, all components that use observable data, should be observers. Because react might schedule re-rendering both on the same stack or a separate stack, and in the latter case accessing an observable won't be tracked, hence not triggering a re-render

@leebenson
Copy link
Author

@mweststrate - thanks for looking. I tried removing .propTypes; same result. The only 'fix' I tried that worked is to wrap the final component in an observer, i.e.:

const Headline = observer(props => (          // <-- now wrapped in 'observer'
  <h1>{props.store.toggle.toString()}</h1>
));

Headline.propTypes = {
  store: React.PropTypes.shape({
    toggle: React.PropTypes.bool.isRequired,
  }),
};

The problem with this approach is that I'm using lots of third-party components in production, so I can't guarantee each will implement observer. My expectation is that I can wrap the first (higher-order) component, and that it will render anywhere in the chain that a change might occur.

What I'm trying to figure out is why it works with npm run storybook (running via Webpack Dev Server ) but not with npm build-storybook (which produces an identical preview.bundle.js to dev, save for different module numbers assigned by webpack.) - is there any obvious reason why React would re-schedule rendering in a different stack in the latter but not the former?

Thanks for your help.

@mweststrate
Copy link

If you have third party components that don't support observer, don't pass them observables, but convert the observables to plain objects in your observer based parent component. That will make the parent responsible for tracking the data instead of the child.

@leebenson
Copy link
Author

yeah, that's actually what I'm currently doing.

I guess knowing that React could re-schedule off-stack, the question is now why it does that in one environment, but not another... any ideas what triggers it?

@leebenson
Copy link
Author

Something else that is a bit freaky too (maybe related to mobxjs/mobx-react#56)

Under npm run storybook, dropping .propTypes actually prevents it from observing:

const Headline = props => (
  <h1>{props.store.toggle.toString()}</h1>
);

/* commenting out .propTypes *stops* it working in dev */
// Headline.propTypes = {
//   store: React.PropTypes.shape({
//     toggle: React.PropTypes.bool.isRequired,
//   }),
// };

So I guess that's exactly what mobxjs/mobx-react#56 relates to -- adding a .propTypes definition causes a re-render (but only in dev; not in production), thus making the component update via npm run storybook but NOT npm build-storybook.

interesting!

@leebenson
Copy link
Author

d'oh, it suddenly makes sense.

.propTypes isn't evaluated in production (it's a dev-only check, right?) So it wouldn't 'observe' the underlying value, and thus wouldn't re-calculate.

So, in this case, .propTypes was causing a side-effect that unintentionally made the component appear to be observing (I guess inside the original call stack passing the prop?), but breaks as soon as .propTypes is dropped in production -- which is what it should do, because the final component isn't an observer.

Makes sense now!

@leebenson
Copy link
Author

I'll go ahead and close this and the related issue, since this seems to be the intentional behaviour and a user-land issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants