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

Different variants on server and client #18

Open alexeyMohnatkin opened 7 years ago

alexeyMohnatkin commented 7 years ago

Server renders variant A, but client script can show variant B after rerender. Also server always returns only one variant until restart. I think it happens because on first render server calls emitter.setActiveVariant and then always get this value even if there's another userIdentifier. The solution is to check userIdentifier before calling emitter.getActiveVariant in getLocalStorageValue() in Experiment component.

Akurganow commented 7 years ago

Has the same problem

durk0 commented 7 years ago

Same problem here

diondirza commented 7 years ago

got same problem even though I have set userIdentifier property. This error occured after I defined multiple experiment variants. Anyone has solved it?

alexeyMohnatkin commented 7 years ago

https://github.com/alexeyMohnatkin/react-ab-test/commit/e2f338b9439fd7cf3afdbf3d7be3379cba19149f Not the best way, but it works. I just haven't much time to rewrite test and make PR.

diondirza commented 7 years ago

@alexeyMohnatkin thanks man, this work for now. You should make a PR for this. This issue really help isomorphic app.

diondirza commented 7 years ago

@alexeyMohnatkin I guess in my end this error still happen even using your patch.

alexeyMohnatkin commented 7 years ago

@diondirza can you create sample project?

peterpme commented 7 years ago

@alexeyMohnatkin thanks so much for giving it a shot! Do you mind creating a pull request with whatever you have? We can work together to make it work :smile:

diondirza commented 7 years ago

@alexeyMohnatkin sorry for late reply, been very busy this week. You can reproduce this error on my repo branch here. You need to retry several times with different browser or incognito mode to be able to reproduce the error.

diondirza commented 7 years ago

@peterpme Very glad to have this repo maintainer back. Do you able to update this repo so it supports latest react version? If you have time to look at my sample repo branch too, there is also a bug in server rendering about this

Warning: setState(...): Can only update a mounting component. This usually means you called setState() outside componentWillMount() on the server. This is a no-op. Please check the code for the Pushtell.CoreExperiment component.

Thoughts?

alexeyMohnatkin commented 7 years ago

@diondirza I don't see any problems with variants. What behaviour do you expect?

diondirza commented 7 years ago

Persistent render result between client and server and I still got this issue on my console sometimes, try running with npm run develop

screen shot 2017-05-06 at 5 54 29 pm
alexeyMohnatkin commented 7 years ago

@peterpme #24 @diondirza it seems that this is something wrong with your project, I can't reproduce this bug on my own

alexeyMohnatkin commented 7 years ago

@diondirza try to rename Experiment 'menu' ;)

diondirza commented 7 years ago

@alexeyMohnatkin what's the problem with my experiment name?

alexeyMohnatkin commented 7 years ago

Sorry, I haven't noticed that you use emitter.defineVariants(). This function causes the bug.

alexeyMohnatkin commented 7 years ago

Actually it's your mistake. You call defineVariants before your app is rendered. Call it in DemoApp/index.js.

diondirza commented 7 years ago

@alexeyMohnatkin I have tried to remove that function in prod app, yet still facing it right now. I think it doesn't make sense that function cause the issue instead of on function that decides what variant to be rendered. Anyway thanks for your help.

durk0 commented 7 years ago

Any progress guys?

wehriam commented 7 years ago

@durk0 can you fork and create a test case that reproduces the issue?

alexeyMohnatkin commented 7 years ago

updated tests in pr

alexeyMohnatkin commented 7 years ago

can anybody merge pull request?

ImanMh commented 6 years ago

pr is merged but I still have the same issue.

stevecaldwell77 commented 6 years ago

FWIW, I wrote a wrapper component to make sure my experiment only gets rendered client-side, so I can use this package in Gatsby. Otherwise I was seeing a flash of one variant (whichever one was chosen when Gatsby pre-built the page), and then to another (whichever ended up getting chosen when rendered in the browser).

Here it is:

import React from "react";
import Experiment from "react-ab-test/lib/Experiment";

export default class extends Experiment {
    constructor(props) {
      super(props);
      this.state = { inBrowser: false };
    }
    componentDidMount() {
        this.setState({ inBrowser: true });
    }
    render() {
        const { inBrowser } = this.state;
        return inBrowser
            ? <Experiment {...this.props} />
            : null;
    }
}