react-server icon indicating copy to clipboard operation
react-server copied to clipboard

ReduxAdapter is not compatible with combineReducers

Open mocheng opened this issue 8 years ago • 5 comments

Glad to know that v0.6.0 has support to Redux, but I found that ReduxAdapter has trouble working with combineReducers which is widely for large applications.

There is an example in react-server-test-pages. The method reduxAdapterReducer is simple and straightforward, but developer generally wouldn't have a giant reducer to cover all, but use combineReducers.

Changing reduxAdapterReducer to be like below and we get an error.

const reduxAdapterReducer = combineReducers({
  routeData: (state, action) => {
    if (action.type === 'ACTION_ROUTE') {
      return action.data;
    }
    return state;
  },
  elementData: (state, action) => {
    if (action.type === 'ACTION_ELEMENT') {
      return action.data;
    }
    return state;
  }
});

The error is below

2017-02-09T02:22:04.974Z - error: [react-server.core.context.Navigator] Error while handling route message=Reducer "routeData" returned undefined during initialization. If the state passed to the reducer is undefined, you must explicitly return the initial state. The initial state may not be undefined., stack=Error: Reducer "routeData" returned undefined during initialization. If the state passed to the reducer is undefined, you must explicitly return the initial state. The initial state may not be undefined

Basically, combineReducers require each reducer to return an non-undefined value on initialization. So, we can change reduxAdapterReducer to have default value as below.

const reduxAdapterReducer = combineReducers({
  routeData: (state={}, action) => {
    if (action.type === 'ACTION_ROUTE') {
      return action.data;
    }
    return state;
  },
  elementData: (state={}, action) => {
    if (action.type === 'ACTION_ELEMENT') {
      return action.data;
    }
    return state;
  }
});

However, since result of reduxAdatper.when got resolved as given Store state is available, the default value makes them resolved immediately. And, there is wierd error generated as below.

2017-02-09T02:24:14.471Z - error: [react-server.core.renderMiddleware] Error with element Connect(BasicComponent)'s lifecycle methods message=Objects are not valid as a React child (found: object with keys {}). If you meant to render a collection of children, use an array instead or wrap the object using createFragment(object) from the React add-ons. Check the render method of BasicComponent., stack=Invariant Violation: Objects are not valid as a React child (found: object with keys {}). If you meant to render a collection of children, use an array instead or wrap the object using createFragment(object) from the React add-ons. Check the render method of BasicComponent.

Perhaps better to detect whether given stateProps is updated than to detect stateProps existence.

mocheng avatar Feb 09 '17 02:02 mocheng

After changing default state of elementData to be string can remove the weird message.

  routeData: (state={}, action) => {
    if (action.type === 'ACTION_ROUTE') {
      return action.data;
    }
    return state;
  },
  elementData: (state='Hello', action) => {
    if (action.type === 'ACTION_ELEMENT') {
      return action.data;
    }
    return state;
  }
});

But, the root issue still exists. The page doesn't wait 500ms to render BasicReduxComponent, but render "Hello" directly as when is done. In browser side, "Hello" is rendered as it is; then it changes to "Element Data" after 3s due to State update. This is not as expected.

mocheng avatar Feb 09 '17 02:02 mocheng

@mocheng This was a problem that I figured would come up and the current solution looks for undefined or null in this case you should be able to set elementData initial state to null and then the when functionality will work. Of course there is a problem of sometimes the resolved value of that state value might actually be null...Banged my head multiple times i still cant think of a more clever way to handle this. The other idea I had was to provide a middleware into the store so that we could instead listen on actions and resolve things as certain actions occur. Would love to hear what you think we could do?

sresant avatar Feb 09 '17 20:02 sresant

@sresant Setting initial state to be null is a solution for me. In case resolved value is actually null, I thought ReduxAdapter.when can check the change of given state, but it might be complicated.

On more thing, I tried to update reduxAdapterReducerwith default value null as below.

const reduxAdapterReducer = combineReducers({
  routeData: (state=null, action) => {
    if (action.type === 'ACTION_ROUTE') {
      return action.data;
    }
    return state;
  },
  elementData: (state=null, action) => {
    if (action.type === 'ACTION_ELEMENT') {
      return action.data;
    }
    return state;
  }
});

It works for routeData. However, I found that BasicReduxComponent gets rendered before elementData is resolved. As a result, the HTML rendered by server doesn't have element data. I haven't dig into this deeper, but obviously the root cause must be in RootProvider.

In the example code, JSX in getElements method is like this.

<RootProvider store={this._store}>
  <RootElement when={whenPromise}>
    <BasicReduxComponent></BasicReduxComponent>
  </RootElement>
</RootProvider>

After I change it to be without RootProvider as below. It works well.

<RootElement when={whenPromise}>
  <Provider store={this._store}>
    <BasicReduxComponent></BasicReduxComponent>
  </Provider>
</RootElement>

Seems existence of RootProvidercould make wrapped children component rendered before when promise resolved.

mocheng avatar Feb 10 '17 01:02 mocheng

@mocheng What is the value of whenPromise? I modified the test page available in react-server-test-pages for the ReduxAdapter (look inside pages/root/reduxAdapter.js for reference) and it seems to work fine with the combinedReducer call the only difference in my code and from what i can see you have is what I passed into when.

sresant avatar Feb 10 '17 04:02 sresant

@sresant Sorry for the inconvenience. I extracted whenPromise from this._storeAdapter.when(['elementData']. So, basically the getElements method is as below.

getElements() {
  const whenPromise = this._storeAdapter.when(['elementData']);
  whenPromise.then(() => {
    console.log('#whenPromise is resolved');
  });
  return [
    <RootProvider store={this._store}>
      <RootElement when={whenPromise}>
        <BasicReduxComponent></BasicReduxComponent>
      </RootElement>
    </RootProvider>,
  ]
}

I also have console.log in render method of BasicReduxComponent. The console.log shows that BasicReduxComponent get rendered before whenPromise is resolved.

In Browser, "View Page Source" also shows no actual payload of BasicReduxComponent. It is the browser side rendering that make "Element Data" filled.

mocheng avatar Feb 11 '17 03:02 mocheng