bdefore / universal-redux

An npm package that lets you jump right into coding React and Redux with universal (isomorphic) rendering. Only manage Express setups or Webpack configurations if you want to.
MIT License
460 stars 48 forks source link

Custom rootComponents in rc28 #78

Closed jazzido closed 8 years ago

jazzido commented 8 years ago

I was using a custom rootComponent, but it looks like that feature was modified in rc28.

Would you provide an example of how they should be specified in the configuration file?

Thanks!

bdefore commented 8 years ago

@jazzido you should be able to use rootComponent as before. If you need a root component that requires serverside libraries such as path you can use rootServerComponent instead. (And conversely rootClientComponent for libraries that rely on webpack aliases.)

Edit: Not true anymore, see https://github.com/bdefore/universal-redux/issues/78#issuecomment-184451392

jazzido commented 8 years ago

I'm a little lost, to be honest. Prior to rc28, my rootComponent.js looked like this (edited for brevity):

export function createForServer(store, renderProps) {
    return new Promise((resolve, reject) => {
        loadOnServer(renderProps, store)
            .then(() => {
                const root = (
                    <Provider store={store} key="provider">
                      <ConnectedIntlProvider>
                        <div>
                          <ReduxAsyncConnect {...renderProps} />
                        </div>
                      </ConnectedIntlProvider>
                    </Provider>
                );
                resolve({ root });
            })
            .catch((err) => {
                reject(err);
            });
    });
}

export function createForClient(store, { routes, history, devComponent }) {
    const component = (
        <Router render={(props) => <ReduxAsyncConnect {...props} />} history={history}>
          {routes}
        </Router>
    );
    const root = (
        <Provider store={store} key="provider">
          <ConnectedIntlProvider>
            <div>
              {component}
              {devComponent}
            </div>
          </ConnectedIntlProvider>
        </Provider>
    );

    return Promise.resolve({ root });
}

I split the two functions into separate files, added their paths to the config file but I'm getting an exception in the renderer.

Would love to use the updated deps in >=rc28, but this is kind of blocking the upgrade.

Thanks!

bdefore commented 8 years ago

@jazzido Sorry for not realizing and mentioning this in my previous comment. After r28, UR now expects the default export to be a function to render. There's no more createForServer or createForClient.

If there's any divergence, you'll need to utilize rootClientComponent and rootServerComponent in separate files that each export default functions. You can see an example of this in how UR now breaks up redux-async-connect support in two, here for server: https://github.com/bdefore/universal-redux/blob/master/src/server/providers/redux-async-connect.js and here for client: https://github.com/bdefore/universal-redux/blob/master/src/client/providers/redux-async-connect.js Let me know if this breaks your implementation.

jazzido commented 8 years ago

Thanks for getting back, @bdefore. So, these are my server and client root components (which are referenced in the config file):

client

// ... edited out imports
export default function createForClient(store, { routes, history  }) {
    const component = (
        <Router render={(props) => <ReduxAsyncConnect {...props} />} history={history}>
          {routes}
        </Router>
    );
    const root = (
        <Provider store={store} key="provider">
          <ConnectedIntlProvider>
              {component}
          </ConnectedIntlProvider>
        </Provider>
    );
    return Promise.resolve({ root });
}

server

// ... edited out imports
export default function (store, renderProps) {
    return new Promise((resolve, reject) => {
        loadOnServer(renderProps, store)
            .then(() => {
                const root = (
                    <Provider store={store} key="provider">
                      <ConnectedIntlProvider>
                          <ReduxAsyncConnect {...renderProps} />
                      </ConnectedIntlProvider>
                    </Provider>
                );
                resolve({ root });
            })
            .catch((err) => {
                reject(err);
            });
    });
}

But I still get the exception that I mentioned in my previous comment, so I suspect the problem is elsewhere.

I'm using a custom html.root, but the exception still happens when using the default (ie: html.js).

Thanks!

bdefore commented 8 years ago

OK. I'll get back to you in 24 hours.

jazzido commented 8 years ago

NP, thanks!

jazzido commented 8 years ago

hey @bdefore , sorry for bugging you. did you have the chance to look into this one?

bdefore commented 8 years ago

Hi @jazzido ... sorry for the delay there. I'm able to reproduce your issue from this branch off of the starter repo: https://github.com/bdefore/universal-redux-starter/tree/root-component-example I'll continue in a few hours.

bdefore commented 8 years ago

@jazzido Sorry I've been completely swamped. I finally got around to investigating further and discovered that in fact I did have some changes to the way that these root components accept their properties. I've updated the root-component-example branch above to a working version. And I suspect that the changes you'll need to make are mostly represented in this commit: https://github.com/bdefore/universal-redux-starter/commit/3c55c77c93a3224fc38075e4a8a0c55ad6358708

Note the changed parameters passed to the function, as well as how the promise is resolved without an object wrapping root. In my example I'm not using the localize component in yours, but that should be able to replace the div wrapper I have in my implementation.

Please let me know if this doesn't work.

jazzido commented 8 years ago

Thanks a lot, @bdefore. That change seems to fix the original bug. However, I ran into a webpack-isomorphic-tools issue after updating universal-redux to 3.0.1. See #86