pushtell / react-ab-test

A/B testing React components and debug tools. Isomorphic with a simple, universal interface. Well documented and lightweight. Tested in popular browsers and Node.js. Includes helpers for Mixpanel and Segment.com.
MIT License
752 stars 112 forks source link

Question: can component inside Variant take props? #7

Closed hlalani closed 8 years ago

hlalani commented 8 years ago

I'm exposing rootState object as a prop to route components (Home component in this example). And I'm trying to pass it down to the component inside Variant (A160331 and B160331 in this example), but it doesn't seem to do work correctly. It only takes the first key value pair inside rootState object.

If I take the A160331 and B160331 outside the Variant, the props work correctly - is this an issue with react-ab-test module or am I doing something wrong here?

const Home = ({rootState}) => {
  return (
    <div>
      <Experiment name="test160331">
        <Variant name="A">
          <A160331 rootState={rootState} />
        </Variant>
        <Variant name="B">
          <B160331 rootState={rootState} />
        </Variant>
      </Experiment>
    </div>
  )
}

Thanks in advance for looking into this. I'm submitting it here instead of SO because I think it may be an issue with the module.

wehriam commented 8 years ago

Yes, components inside of <Variant> can take properties.

Variant doesn't do much (take a look at the source) so I'm not sure where it would be interfering.

What is the rootState object in this situation? Can you paste in the entire component so I can see the lifecycle?

hlalani commented 8 years ago

Essentially rootState is like the state object in Redux, except I'm using RxJs to implement it:

state$.subscribe(change => {
  const createElement = (Component, props) => {
    const rootState = change
    return <Component {...props} rootState={rootState} />
  }
  render((
    <Router routes={routes} createElement={createElement} history={browserHistory} />
  ), global.document.getElementById('app'))
})

On every state change event, routes are updated and re-rendered.

The code in the previous comment is the entirety of the Home module - A160331 is a variant component I want to test and in it I'm having trouble rendering another subcomponent AddToCartButton:

const A160331 = ({rootState}) => {
  return (
    <div>
      <AddToCartButton rootState={rootState} products={products} className="mx1 my2" />
    </div>
  )
}

This component takes the rootState which includes "cart" state. If for example I click this button, it'll send a state change event, which will re-render the shopping cart with an added product. If I take the A160331 component outside of Variant, logging rootState inside A160331 might show:

{
  countryCode: 'US',
  cart: {
    cartItems: [{a: 1}, {b: 2}]
  },
  notificationBar: {visible: true},
  ...
}

But when inside Variant, A160331 will log out the following for rootState prop:

{
  countryCode: 'US'
}

Also, I noticed that debug panel still shows in production, even though I'm using Heroku to deploy it and build log shows that NODE_ENV=production.

wehriam commented 8 years ago

Using a function that returns a component rather than a React component class may not work in the way you're expecting it to.

I'm having a hard time understanding your code, but take a look at the <CoreExperiment> source. My guess is that <A160331> is rendering earlier in the lifecycle when inside of the <Experiment> and that changes to the rootState object are not triggering a render.

You might consider refactoring <A160331> and <Home> into React component classes so there's less ambiguity about how state (and in this example, changes to rootState) affects renders. Keep an eye on componentWillReceiveProps.

wehriam commented 8 years ago

To hide the debug panel when NODE_ENV=production you'll need to use envify for your builds.

hlalani commented 8 years ago

Thanks for pointing out the fix with debug panel.

As for the prop issue, I think you're right in suspecting that the state is not updating, but I wasn't able to pinpoint why. My best guess is that how I'm handling state with RxJs doesn't play well with how react-ab-test is handling state with setState because if I simply take the component out of Variant, everything works as expected (I tried converting the components to React classes, but it didn't do anything).

I may debug the issue later, but for now, I'm using a hack - saving rootState to localStorage on state change and using that inside the components to bypass the state update issue.

Thanks for looking into this. I'm closing this issue.