Skip to content
This repository has been archived by the owner on Jan 22, 2020. It is now read-only.

Breaking the encapsulation of Client #50

Open
dglozic opened this issue Jul 5, 2015 · 13 comments
Open

Breaking the encapsulation of Client #50

dglozic opened this issue Jul 5, 2015 · 13 comments

Comments

@dglozic
Copy link

dglozic commented Jul 5, 2015

I have noticed the following line in index.js the examples:

var Client = require('react-engine/lib/client');

When I saw this, I thought that it is bad form because it makes assumption about internals of react-engine. I 'fixed' it like this:

var Client = require('react-engine').client;

However, this didn't work for browserify - I got weird errors about 'not being able to find react module' in the browser console.

I still think that we should not reach into 'lib' folders of react-engine, but somehow this works for me, albeit leaving me with mixed feelings :-).

@samsel
Copy link
Contributor

samsel commented Jul 13, 2015

@dglozic very true :) I am not happy about the client file design. thinking of rewriting it to avoid all those unwanted dependencies that this api, by design brings in.

@samsel
Copy link
Contributor

samsel commented Jul 13, 2015

@dglozic have you encountered anything else good for this case?

@dglozic
Copy link
Author

dglozic commented Jul 13, 2015

Didn't look as much - after your blog I liked this one so much I stopped searching :-).

One thing I noticed as a problem is handling data stores (flux). If I define a data store, it will need to initialize in a similar way as state for components after rendering on the server. Right now, I am initializing data stores in client loading callback, and it works, but it would be better if I could define a store, initialize it in the express controller, let the views use it for server rendering, then have a way of sending the data in the store to the client.

That would really round it up.

@samsel
Copy link
Contributor

samsel commented Jul 13, 2015

but still you need to bootstrap the store on the client right?

@dglozic
Copy link
Author

dglozic commented Jul 13, 2015

Another minor thing is that we are rendering our global header on the server by making a network call to the header service, then inlining the HTML snippet using 'dangeriouslySetInnerHRML'. This information is passed into a view through view props, and unfortunately even though it is never used on the client, it is transferred to it. Even worse, I ended up URLencoding the value because without it will mess up the resulting HTML.

Ideally I should be able to say which properties to be ignored and not sent to the client.

@samsel
Copy link
Contributor

samsel commented Jul 13, 2015

we already omit some stuff. would it help if we make this configurable? so that you can add more stuff as part of the setup? https://github.com/paypal/react-engine/blob/master/lib/server.js#L89

@dglozic
Copy link
Author

dglozic commented Jul 13, 2015

Oh, that would be great!

Now, if we provided the solution for Flux stores as well, it would be perfect :-).

Essentially an opportunity for the data store to serialize its data on the server, and then initialize from it during the Client booting, but without burdening top level view's props.

@samsel
Copy link
Contributor

samsel commented Jul 13, 2015

i quite don't understand the flux related ask. can you explain a bit more?

@deowk
Copy link

deowk commented Jul 14, 2015

@samsel : I am busy building an app with react-engine and redux, and I had a bit of trouble with this particular issue as well. Basically what you need to do is instantiate a redux instance server side and fire an action to populate your stores with the data that you fetched server-side, you then need to get that data to your client somehow because it will need to be set as the initialState of your client side stores - with react-engine you can just pass it through props or use client.data(). The only snag with this approach is that both server-side and client are actually using the same redux instance and it would be very useful to be able to have a render method for server and client separated out. Redux seems to be picking up some momentum and it's definitely something you should think of integrating well with.

@samsel
Copy link
Contributor

samsel commented Jul 14, 2015

@deowk thanks for the feedback. can you answer the questions below so that I can get some more clarity?

The only snag with this approach is that both server-side and client are actually using the same redux instance

  • how do you reuse the instance normally (lets say without react-engine in your app)?

it would be very useful to be able to have a render method for server and client separated out

  • the server and client render methods are different? can you be a more specific on the need?

client: https://github.com/paypal/react-engine/blob/master/lib/client.js#L46
server: https://github.com/paypal/react-engine/blob/master/lib/server.js#L97

@dglozic
Copy link
Author

dglozic commented Jul 15, 2015

@samsel: the Flux issue is that in the component hierarchy, you have a top-level controller-view that has a state, and then the children receive some fraction of that state as props. Normally top-level view will listen to a store and update when the store changes. react-engine sends the props to the client but there is no way to send the data to initialize the store without making an xhr from it. It would be nice to be able to piggy-back on the state sending mechanism and carve out a section for data that stores can initialize from.

@deowk
Copy link

deowk commented Jul 16, 2015

how do you reuse the instance normally (lets say without react-engine in your app)?

let's say I want to trigger an action on the redux instance when a route transition happens BUT only the client side, normally without react-engine I would do something like this.

server-side.js

const redux = createRedux(api);

  ReactRouter.run(routes, location, async (err, routerState) => {
    try {
      if (err) {
        throw err;
      }

      const { params, location } = routerState;
      const prepareRouteMethods = routerState.components.map(component =>
        component.prepareRoute);

      for (let prepareRoute of prepareRouteMethods) {
        if (!prepareRoute) {
          continue;
        }

        await prepareRoute({ redux, params, location });
      }

      const body = React.renderToStaticMarkup(
        <Provider redux={redux}>
          {() => <Router {...{ ...routerState, location, history }} />}
        </Provider>
      );

      const initialState = redux.getState();

      res.render(layout, { ...payload, body, initialState });

    } catch(err) {
      res.status(500).send(err.stack);
    }
  });

client-side.js

const redux = createRedux(api, __INITIAL_STATE__);

Router.run(routes, Router.HistoryLocation, function(Handler, locationState) {
   redux.dispatch(routeLocationDidUpdate(locationState))
   React.render(  
    <Provider redux={redux}>
      {() => <Handler />}
    </Provider>,
    document.getElementById('main')
   );
});

but react-engine abstracts this part away and I don't want to go digging in /lib to modify this plus then I would also need to modify the server.js file to pass its own instance of redux because at the moment I am doing that in app.js and both the client and server are touching this file hence both the server and client are using the same redux instance.

export default class App extends React.Component {
  render() {
    const redux = createRedux(api, this.props.initialState);
    return (
        <Provider redux={redux}>
        {() =>
          <Layout {...this.props} >
            <Header {...this.props}/>
            <Router.RouteHandler {...this.props} />
            <Footer {...this.props} />
          </Layout>
        }
        </Provider>
    );
  }
}

I hope this explains my particular issue properly, Mainly the problem is that I need finer grain control over the server and client render methods without changing stuff in /lib

@andytompkins
Copy link

I think I am having a similar issue. I want to use Alt as my flux framework, and Alt provides functions bootstrap and takeSnapshot that will save/restore the state of all the stores. If I do this before res.render, then stores that are initialized via inclusion from views will be missed. I need a way to save state after the render of the component, but before the html is sent to the client. What I've done is to modify my server like this

let engine = renderer.server.create({
    routes: Routes,
    routesFilePath: path.join(__dirname + '/routes.jsx'),
    serialize: function() {
        return alt.takeSnapshot();
    }
});

Then I modified the react-engine server like this:

// render the componentInstance
      html += React.renderToString(componentInstance);

      // serialize app state and save to data
      if (createOptions.serialize && typeof createOptions.serialize == 'function') {
          data.appState = createOptions.serialize();
      }

      // state (script) injection
      var script = format(TEMPLATE, Config.client.markupId, JSON.stringify(data));
      html = html.replace('</body>', script + '</body>');

And in my client I do this:

var data = Client.data();
alt.bootstrap(data.appState);

Is this reasonable? Is it the right approach? Is this something that would be considered in PR form?

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

No branches or pull requests

4 participants