Make reflux isomorphic friendly

spoike commented 9 years ago

Feedback I receive around suggest that reflux needs to be more friendly for isomorphic projects, i.e. same actions and stores can be run both client-side and server-side.

The goal with this issue is how can we achieve this in reflux and still keep a simple API?

Suggestions and PR's are welcome.

spoike commented 9 years ago

Example of isomorphic project using refluxjs and react-async that recently came to my attention:

https://github.com/chadpaulson/react-isomorphic-video-game-search

WRidder commented 9 years ago

Something I'm fiddling with as well. In my case it would mostly mean making sure the initial data in the stores is available. No idea yet how to approach this effectively so looking forward to the discussion.

Janekk commented 9 years ago

@WRidder - so basically it would mean initializing stores with the data already fetched on the server? For example for the ToDo app it would be something like that?

<body>
  ...
  <script>
  Reflux.initStore(TodoStore(
    {"entries":[
        {"Text":"Make to do list","CreatedAt":"2014-11-27T13:20:13+02:00"},
        {"Text":"Check off first thing on to do list","CreatedAt":"2014-11-27T13:25:13+02:00"}]
    }));

  Reflux.initStore(UserStore(
    {"name":"Guest","loggedInAt":"2014-11-27T12:20:13+02:00"}));
  </script>
</body>

WRidder commented 9 years ago

@Janekk: In my specific case, for now at least, that would work indeed. One of the challenges is making sure what stores to initialize given a certain route, you'll need to have that data ready and put in the stores before you can call renderComponentToString which is of course done on the server side.

I'm pretty sure there will be a lot of cases where this approach won't suffice though.

spoike commented 9 years ago

I'm entertaining myself with the idea of a hydrate/dehydrate function on Reflux... That way you can do this on server-side:

var initialData = getInitialData(); // get from req and other data sources
Reflux.hydrate(initialData);
var string = React.createComponentFromString(App(initialData));
var renderedData = Reflux.dehydrate();

And on client-side:

var initialRenderedData = getInitialData(); 
    // get e.g. from document that's been rendered
    // serverside (i.e. renderedData from previous 
    // example)
Reflux.hydrate(initialRenderedData);
React.createComponent(App(initialRenderedData), element);

This also means that all actions and stores needs to be addressable so that Reflux knows where to put data for corresponding stores. Currently stores do not have a name, which means breaking API change (or having it optional and extending the API).

Comments?

WRidder commented 9 years ago

Alrighty, challenge here is determining your initial data if I'm correct. Hydrate does indeed need to know where to put this data but why does the App(initialData) need to be there if the stores are already hydrated?

I like the idea of hydrating the application on the clientside this way.

spoike commented 9 years ago

You're right @WRidder. If users are using the getInitialState parts of reflux, you won't need to add initial props to the App component.

appsforartists commented 9 years ago

I take it you guys have already seen this, but if you haven't, it shows the challenges Yahoo! had to overcome to make Facebook's Flux implementation server-safe.

gpbl commented 9 years ago

I've been playing around with the problem since the first days of react and I still have to find the right way to solve it. The main challenge is how to get the initial data having a dispatcher running in different contexts: per request on the server or per session on the browser. It comes a bit difficult especially when the data must be fetched async, since on the server the components cannot be updated once they have been rendered - here I guess it comes in play react-async, but I'd like to skip it since it does not feel right.

The yahoo developers with fluxible-app actually solved the problem, but to make it working I had to adopt their own flux architecture. It also seems to be at an early stage, the whole app didn't look simple and elegant as with refluxjs and react-router.

Let say I have my router on the server which exposes the entry point for the application:

// express app
Router.run(routes, req.url, function (Handler, state) {
    fetchData(Handler).then(function(initialState) {
        var markup = React.renderToString(<Handler initialState={initialState} />);
        // dehydrate things and add them to markup
        res.send("<html><body>" + markup + dehydratedState + "</body></html>");
    )
  });

How could fetchData() work with reflux?

appsforartists commented 9 years ago

One of the ideas I've been playing with is having a function that populates your stores based on some state, since the data you need to fetch should be wholly dependent on the URL (and maybe the user's cookie).

So, @gpbl's example could become something like:

// express app
Router.run(
  routes, 
  req.url, 

  function (Handler, state) {
    populateStoresForState(state, req.cookies).then(
      function (dehydratedStores) {
        res.send(
          `<!DOCTYPE html>
           <html>
             <head>
               ...
               <script>
                 Reflux.rehydrate(${ dehydratedStores });
               </script>
             </head>

             <body>
               ${ React.renderToString(<Handler />) }
             </body>
           </html>`
       );
    )
  }
);

I think the biggest challenge will be finding the optimum API that allows Reflux to be instantiated, so each request can have its own instance of Reflux. (Otherwise you have to worry about leaking data across requests and race conditions.) The app developer could then back the stores with memcached on the server-side, so data that should be shared between requests can be explicitly cached (rather than implicitly reused between requests).

Laiff commented 9 years ago

I've be playing with Reflux on server side and now implement API like this.

Reflux.run(function(refluxContext){
// refluxContext - populated with stores and actions. 
// And rehydrate data without trigger change actions. 
// Usage refluxContext.store(AuthStore) 
  React.withContext({reflux: refluxContext}, function() {
    Router.create(routes, location).run(function(Handler, state){
      // some play with state from router
      React.render(<Handler {...} />)
    })
  })
})

This refactoring is very complicated, because it requires implement lifecycle for stores and actions, and plugin system for reflux. I'm oriented on ReactComponent creation and initialization. Now I working on basic API for inner infrastructure.

WRidder commented 9 years ago

At the moment I managed to make my PoC application (nearly) fully isomorphic. Time will tell if the approach is manageable.

A normal lifecycle looks like this:

First load on browser hits Express endpoint
Instantiate the application (which has a library interface so NodeJS can use it as well) using this method.
In the node environment, all resource loading is synchronous which makes sure the stores are filled before component rendering occurs. I use a data interface to manage synchronous data fetching in a nodejs environment.
Output result in a html template and serve this to the user.
On clientside, the app instantiates and takes over the page (for which react does not need to rerender).

To make sure the data which is needed for a certain path I can do a 'profile run' of the applcation once for a path. This allows us to check what data is required.

This is still heavily Work in Progress. However, for those who'd like to try: React-spa.

esamattis commented 9 years ago

I think the biggest issue using Reflux in the server is how to know which stores are related to which url. Or even which components? If we knew the the components I guess we could add static store getters from the Reflux.connect(...) mixins.

Another big issue is how to manage store states within concurrent requests.

Currently all I can come up is to do a profile run as @WRidder suggested and mangle getInitialState methods like this:

During the profile run I can listen which stores get called using the getInitialState method
Asynchronously fetch data for the stores based on the url params
During the next tick
1. Mangle the getInitialState methods to return the fetched data
2. Rerender the components
3. Restore getInitialState methods to their normal behaviour

Pretty hacky.

willembult commented 9 years ago

Although I've only been working with React/Reflux for a week, this is something that I can already see lurking behind the horizon.

Seems to me we have a few challenges:

How do we know what state to adopt on app load? On server this will be completely determined by the URL and cookies, as noted by @appsforartists. That is, if our server is state-less, which generally seems like a good idea :) If we want a stateful server, we'll need a way to store/restore state between requests, but for the purpose of this discussion I'd regard that a separate concern. On the client, we should take over from the server, so when we render state A on the server, we can encode and transfer that state, and the client could inject that state A (i.e. hydrate/dehydrate).
How do we deduce state from URL and cookies (on the server)? Luckily, in our architecture state is neatly contained in a strict subset of our application: the stores. It seems to me loading the state is already contained naturally in the application logic, through getInitialState. However, since state is often loaded asynchronously, we have to wait for the state to be stable before we render / dehydrate. We just need to know what stores to listen to, which to me seems should be exactly the ones on which getInitialState() is called. A profile run as @WRidder suggested seems quite imposing; what if we simply recorded the calls to getInitialState()? We'd then need to be able to address all stores in our app, but @spoike brought up the point earlier that we'll need addressable stores to inject state as well anyway. Maybe it doesn't have to be a breaking interface change, though...
How do we adopt a certain state (on the client)? If we can dehydrate state from loaded stores into a simple object and send it over to the client, how does the client load it into its stores? This seems less of an issue, although if we're not careful it could break the unidirectional data flow. I think we should be careful to introduce setState or hydrate methods directly on the store. Even having getInitialState() responsible for loading data imposes slightly on the model, in my view, but alas.

Perhaps all of this could look something like this?

Reflux.ListenerMethods.fetchInitialState = function (listenable, defaultCallback) {
  var me = this;

  defaultCallback = (defaultCallback && this[defaultCallback]) || defaultCallback;
  if (_.isFunction(defaultCallback) && _.isFunction(listenable.getInitialState)) {

    if (!listenable.initialStatePromise) {
      data = listenable.getInitialState();
      if (data && _.isFunction(data.then)) {
        listenable.initialStatePromise = data;
      } else {
        listenable.initialStatePromise = Q(data);
      }
    }

    listenable.initialStatePromise.then(function() {
      defaultCallback.apply(me, arguments);
    });
  }
};

/*
 * Creates a StoreManager that can hydrate/dehydrate
 * all the stores supplied in this.stores.
 *
 * Usage: Reflux.createStoreManager({
 *   stores: {SessionStore: require("./sessionStore")};
 * });
 */
Reflux.createStoreManager = function(definition) {

  function StoreManager() {
    this.initialPromises = {};
  }

  StoreManager.prototype.findInitializedStores = function() {
    var namesAndPromises = {names:[], promises:[]};

    for (var storeName in this.stores) {
      store = this.stores[storeName];
      if (store.initialStatePromise) {
        namesAndPromises.names.push(storeName);
        namesAndPromises.promises.push(store.initialStatePromise);
      }
    }

    return namesAndPromises;
  }

  /**
   * Returns a promise with the initial state of all
   * stores in the StoreManager for which getInitialState()
   * was called.
   */
  StoreManager.prototype.dehydrate = function() {
    var namesAndPromises = this.findInitializedStores();

    deferred = Q.defer();

    // a bit dangerous, we need to be sure earlier callbacks
    // don't return anything other than the original arguments
    Q.all(namesAndPromises.promises).then(function(states) {
      var i = 0,
        state = {};

      for (;i<states.length; i++){
        var name=namesAndPromises.names[i];
        state[name] = states[i];
      }

      deferred.resolve(state);
    }).catch(function(error) {
      deferred.reject(error);
    });

    return deferred.promise;
  };

  /**
   * Replaces the getInitialState() method for all stores
   * in the StoreManager with a function that simply returns
   * the state as contained in the supplied dehydrated state
   */
  StoreManager.prototype.hydrate = function(state) {
    for (var storeName in state){
      if (this.stores[storeName]){
        storeState = state[storeName];
        // replace getInitialState to return injected state
        this.stores[storeName].getInitialState = function() {
          return function() {return state;};
        }(storeState);
      }
    }
  };

  _.extend(StoreManager.prototype, definition);

  return new StoreManager();
}

Usage would be like so:

storeManager = Reflux.createStoreManager({
  stores: {
    SessionStore: require('./session/store');
  }
});

// load the initial state from document here
// need to do this before app/stores are initialized
initialState = {
  SessionStore: { user: null } 
};

storeManager.hydrate(initialState);

// Load / initialize app here as normal

// dehydrate at the end, when all stores have been initialized
storeManager.dehydrate().then( function(state) {
  // render with state
});

spoike commented 9 years ago

I'm starting to wonder if we shouldn't make the a state variable in store mandatory, much like React components, for easy "serialization" when hydrating/dehydrating (and also for an eventual builder pattern #149).

I'd like to avoid adding more third party libraries into Reflux, since I'm pretty sure everyone will be using their own favorite promise library (i.e. q.js or bluebird) and I don't want to dictate that. We can just use a callback as parameter to dehydrate:

storeManager.dehydrate(function(dehydratedState) {
  // render with dehydratedState
});

And if anyone wants to use it with promises instead, they can just extend/monkey patch Reflux with a custom function. Example with Q:

Reflux.dehydrates = function() {
    return Q.promise(function(resolve) {
       Reflux.dehydrate(resolve);
    });
};

// Usage:
Reflux.dehydrates().then(function(dehydratedState) {
    // render with dehydratedState
});

appsforartists commented 9 years ago

Promises are supported in every major browser, except IE. You could just use native Promises and tell people who need to support IE to include a polyfill. That seems like a sane solution.

You could also return a thenable from dehydrate and let people wrap it in Promise.resolve for error handling.

andyl commented 9 years ago

"I'm starting to wonder if we shouldn't make the a state variable in store mandatory, much like React components"

I am a fan of this - for easier serialization and testing. Having a standard store variable will make Reflux easier to understand for newbies.

But I believe the store variable should be called data instead of state. Because 1) store objects are called "data stores" not "state stores", and 2) it makes the terminology distinct from React components. Stores manage data, Components manage state and props.

willembult commented 9 years ago

"I'd like to avoid adding more third party libraries into Reflux," @spoike I agree too much dependency on 3rd party libraries should be avoided, and that we can easily do without here. We could use any of native promises, thenable functions, or plain callbacks.

"I'm starting to wonder if we shouldn't make the a state variable in store mandatory" @spoike If we want to have access to full state for dehydration, instead of getInitialState(), then I agree it would be useful for dehydration. If we don't, I guess I don't really see the need, but I do like the consistency with state in React components, so I'm in favor.

"But I believe the store variable should be called data instead of state." @andyl I actually think components are generally not in the business of managing state. In my view, components should be mostly stateless (except for some local view-specific state), and stores actually manage application state. But the state of a store is defined by the data in the store, so having the field named data seems fine as well, and it fits the "datastore" label better. Not sure...

The core assumption I made is that state to be dehydrated server-side should always be present in the result from getInitialState(). Maybe this is somewhat restrictive, but it comes with the additional nice property that we can tell exactly which stores have been initialized and to include in dehydration. What are people's thoughts here?

appsforartists commented 9 years ago

The core assumption I made is that state to be dehydrated server-side should always be present in the result from getInitialState()

@willembult How would that work with asynchronous data? It seems to me that the flow would be something like:

Server-side request handler calls populateStoresForState(request.url, request.cookies)
populateStoresForState invokes the actions that tell the stores to load data from {memcached, REST API, database}
When the a store's data is ready, it fires the relevant event to tell the app
populateStoresForState listens for these events, and when all the data is available, it resolves itself
The server's request processor renders the app, passing the appropriate state into the component tree

If the store is responsible for fetching its data (which seems sensible at first blush), I don't know how you'd satisify this requirement that Store.getInitialState has all the state any server-side rendering would need.

willembult commented 9 years ago

@appsforartists Ah yes, indeed, asynchronicity makes life difficult, see my note above. I think the flow could be like this:

Server-side create a StoreManager with a reference to all our stores (if it wants to dehydrate).
Handle request as normal, while we keep track of getInitialState results (which could be promises, indeed). The stores would maintain these internally, possibly based upon a flag that StoreManager can set on the store.
StoreManager.dehydrate() returns a promise that resolves by combining all the states (promises) returned by all the getInitialState calls.

In this case, we know exactly which store states map to a certain route, because they're exactly the ones who were initialized logically by the app given the route. We don't have to satisfy the requirement of Store.getInitialState() having all the state, as long as we keep track of the promises so we can combine the results asynchronously.

I would be hesitant to have the stores report to the app that their data is ready, because I think that breaks separation of concerns and introduces circular dependencies. Or we could use a getInitialState action instead, avoiding circular reference that way, but since it only ever executes once, it seems like it's not quite a right fit either. Instead, if stores just keep track of their initial data themselves (possibly optionally, when informed to do so), we can have an external entity inspect and dehydrate that data.

appsforartists commented 9 years ago

I haven't done much Flux work yet, but I was under the impression that having a new event enter the system when async data loads is pretty par-for-the-course in Flux.

Think about it this way: In a typical Flux app, the user can do something in a component that triggers an action (e.g. changing the currentIndex in a list). The store would then check to see if it has the data for that item in cache, and if it doesn't it would go fetch it. When it has the data, it notifies the root component that there's new data and it's time to redraw. From what I've read about Flux so far, that seems like a pretty standard flow.

Conceptually, all we would need to do to have a really clean isomorphic architecture is move the initial trigger out of React and into its own function. If you think of navigating to a URL representing a user performing an action (just as clicking a component would be), you can see how it's appropriate to invoke an action from the initial URL. You can transform the URL parameters into a a promise that is resolved when all the stores are filled. When that promise is resolved, render the app.

If you call the transformation populateStoresForState, you have the data flow I described earlier. As I disclaimed, I'm not yet a Flux expert, but I don't see how that is any more circular than a typical Flux flow. Can you help me understand?

willembult commented 9 years ago

@appsforartists Good point. I'm by no means a Flux expert either, so I appreciate the discussion here.

The circular reference I was referring to would be of the app including stores, and the stores having to include the app because they want to inform the app on completion of initial data load. Like I said, an action/event would eliminate that, which is what I think you're suggesting.

What I failed to properly realize before, is that getInitialState gets called from a React component, which we shouldn't render before we have any state... What you're proposing, if I understand it correctly, is to take the initiation out of the components and into an external controlling entity. So if before we had:

MainAppComponent (or Router).render() -> SomeComponent.componentDidMount() -> Store.getInitialState()

Now we'd have:

populateStoresForState(route) -> Store.getInitialState() -> MainAppComponent.setState() -> MainAppComponent.render()

Is something like below what you mean?

function populateStoresForState(uri, cookies) {
  SessionActions.initialize();
  SessionStore.listen(function(state) {
    // yay, we have state
  });
}

SessionStore = Reflux.createStore({
  init: function() {
    this.listenTo(SessionActions.initialize, this.getInitialState);
  }

  getInitialState: function() {
   getWhatINeedAsync().then(function (result) {
      SessionActions.initializeCompleted(result);
      this.trigger(result);
    }).catch(function (error) {
      SessionActions.initializeFailed(error);
    });
  }
});

If I understand correctly (and I'm not certain I do), in React, components dictate what data to load, which (server-side) depends exclusively on the URL and cookies, typically through a Router component. If we took the initial trigger out of the components, aren't we breaking encapsulation by putting logic which belongs in the components outside? We'd now have to determine centrally which stores to load, based on the URL, in populateStoresForState. Isn't it the components' job to decide what stores to initialize? Suppose I had a reusable component with its own store and I wanted to include it in my app, do I now also have to modify populateStoresForState? Basically, how can populateStoresForState figure out what stores to populate, without delegating to the components?

I do see the problem with the async loading of initial state that you're referring to, though. The initial loading gets triggered by render(), which is precisely what we shouldn't call before we're ready. Interesting...

appsforartists commented 9 years ago

This deserves a more complete reply than what I can send from my phone, but here it goes:

I think you get the gist of what I'm proposing. There are a few important points to remember:

Server-side rendering is most important for things like search engine optimization, where the async data you need is wholly dictated by the URL. A user entering a URL is showing intent to ingest content, so it's as appropriate for server-side URL-parsing logic to encode that intent into an action as it would be for a component that allowed the user to request by clicking to invoke it into an action. It's effectively creating another UI for controlling app state, only this one doesn't need React.
populateStores is userland code (not part of Reflux), so if someone feels strongly that the components, rather than the stores or the server, should be kicking off the requests, there's nothing keeping them from developing a convention for their app, like "each component has a static method for kicking off its data fetching" and walking the component tree on request. In fact, react-router had an example of this in their docs.
The user (through either in-app UI or the URL) is responsible for what should be loaded; the stores should be responsible for how to load it. The component shouldn't know or care if I'm using memcache, an external API, or a database to back my stores, and I should be able to utilize different techniques on the client and server. Having data passed in externally as props enforces this.

Since for at least many use cases, inferring what data needs loading from the URL is a good solution, I think we should reduce the scope of this to strictly what's needed to support that: namely, making Reflux instantiable and implementing dehydration. If it turns out we need a more opinionated solution later, let's address that in a separate PR. For now, let userland code worry about what gets loaded; we just need to be concerned with how it does.

Sent from my phone

xavierelopez commented 9 years ago

Great discussion going on here. If I may add something, I'd say that to me, refluxjs shouldn't care about how isomorphic my app is, instead, it should provide me with helpers or mixins to facilitate different store initialization approaches as needed.

One seemingly simple approach that comes to mind is having the component take care of the store initialization via props. How the prop is serialized and available to the component is probably best left for some other library to implement or help with (using something like express-state, or manually adding markup with serialized data).

Assuming we're talking about a top level component that is rendered on both server and client, and that this.props.data is all the data we need, then this is how a getInitialState would work on both stacks.

getInitialState() {
  // this.props.data is prefetched data
  // loadData is a synchronous action
  DataActions.loadData(this.props.data);
  return dataStore.getState();
}

or in a perhaps more pragmatic but less idiomatic way

getInitialState() {
  // this.props.data is prefetched data
  return dataStore.setInitialState(this.props.data).getState();
}

willembult commented 9 years ago

@appsforartists thanks for your comments, it helped me understand better what you mean. This especially struck a chord: The user (through either in-app UI or the URL) is responsible for what should be loaded; the stores should be responsible for how to load it.. I agree and I think this is important to keep in mind. Stores should be doing the loading, however they see fit, which on server could be different than on client. The load by URL is just another user event that informs us what to load. I think we're on the same page so far, right?

I guess I'm still somewhat confused as to why this is different on the server than on the client, and why we would need extra code to figure out what state belongs to a route. If I refresh the app in a browser, my app is also just responding to the current route (user event), to figure out what data (state) to ask the stores to load. How is this scenario really different from server rendering, other than the fact that we have to wait to render until all data is ready?

Maybe I'm approaching data initialization in a wrong way in general, and that's where my confusion is stemming from. Let's take a contrived example: say somewhere in my app I have a ProductList component. When my app decides to render that component (in response to the URL load), it will go and ask the ProductStore to load all the products (without caring how), and when that's finished it will trigger a re-render. Isn't that how we would typically initialize store data?

The only difference I see here is that on the server we have to hold off on rendering until all that data is loaded. Why do I have to add code to map routes to state once more and have my stores load the data accordingly (e.g. walk component tree in populateStores)? Isn't that behavior fundamentally already there (albeit async)?

willembult commented 9 years ago

@xavierelopez isn't that shifting the responsibility of managing state from the store to the components? Shouldn't the components simply ask the stores to load the data (intent through actions), and have the stores actually get that data however they want?

xavierelopez commented 9 years ago

@willembult, it doesn't shift it entirely, only during startup. Once the component triggers an action with the data based on its props, then the store takes over managing the state and the component only uses that.

Ideally that initial action is what actually triggers the store to load its data asynchronously, not what transports synchronously it to the store. But since we don't have the benefit of async rendering on the server, this is a way to keep the actual component, action, and store code simple, while adding more responsibility to the server router (by fetching the data based on the url, cookies).

appsforartists commented 9 years ago

@willembult I wouldn't walk a component tree at all. I was only pointing out that there's nothing preventing someone who wanted to manage all state from the components to do so.

In my mind, any async data you want to show on the page should be either a) global, or b) route-specific. If we accept that presumption to get something working in the simple (and general) case, you don't have to care at all about the components until after you have your data.

In your example, I imagine the route would look something like this: /products/:categoryKeyName/. When the user navigates to /products/bikes/, your router would parse that and send {"categoryKeyName":"bikes"} to you.

populateStores would look at that and know that it needs to lookup "bikes" in your {memcache, database, api}. That's a route-specific lookup. It would also load the category list because every page on your site needs a category list (the global lookup). When both the route-specific and global lookups are complete, it can resolve its promise with their results.

When that's resolved, the component tree is rendered to a string and returned to the client.

Notice that nowhere in the above flow did we discuss Reflux, because all that stuff happens outside it. For Reflux, we just need to make sure that we can instantiate per-request actions and stores, and serialize/deserialize them accordingly as an app instance transitions from running in a server request to on the client.

Does that make it more clear?

willembult commented 9 years ago

@appsforartists Sure, that all makes complete sense, and that'll work just fine. The point I'm debating is this:

populateStores would look at that and know that it needs to lookup "bikes" in your {memcache, database, api}

If populateStores has to know that it needs to lookup "bikes", that means we need to write code (outside of Reflux, as you point out) that acts upon receiving {"categoryKeyName": "bikes"} from the router and loads the corresponding products into the store. That's fine, but the component that already logically responds to the same route (lets call it ProductItemList), would in the current / client-side case already request my store to do the exact same load when it gets initialized through componentWillReceiveProps or the like. The store may execute that load request differently on client and server side, but the component doesn't have to care and fundamentally the load intention is the same.

Let's apply the same example to a client-side page reload. Now we don't call populateStores, but ProductItemList.componentWillReceiveProps gets the categoryKeyName passed from the router just fine, and will call ProductActions.loadCategory("bikes"), just like populateStores would. Isn't your suggestion to duplicate that logic? The mapping from /products/:categoryKeyName to ProductActions.loadCategory(categoryKeyName) would now live in 2 separate places, wouldn't it?

appsforartists commented 9 years ago

Why have the store initialized in componentWillReceiveProps at all? If the store's data is dependent only on information you know before you render, you can share populateStoresFromState across both runtimes and have it call the relevant Reflux actions to write data to the store:

// server

var handleRequest = function (connection) {
  ReactRouter.run(
    routes,
    connection.request.path,

    (Handler, state) => {
      populateStoresFromState(
        state
        connection.request.cookies

      ).then(
        populatedStores => {
          connection.html(
            [
              "<!DOCTYPE html>",

              React.renderToStaticMarkup(
                require("Scaffold.jsx")(
                  {
                    "dehydratedStores": Reflux.dehydrate(populatedStores),
                    "body":             {
                                          "__html":   React.renderToString(
                                                        <Handler/>
                                                      )
                                        }
                  }
                )
              )
            ].join("\n")
          );
        }
      )
    }
  )
};

// client
ReactRouter.run(
  routes,
  ReactRouter.HistoryLocation,

  (Handler, state) => {
    populateStoresFromState(
      state, 
      document.cookies

    ).then(
      populatedStores => {
        React.render(
          <Handler/>,
          document.body
        );
      }
    );  
  }
);

Then, you can either have the data passed as props/context from the root handler to its children, or you could just have the components themselves look at the stores and say "what's the current value for this variable?". But, I'm not convinced the components should be in the business of telling Reflux what data should be available. Instead:

1) The end user visits a URL. 2) The router looks at the URL and tells the app there's new route state. 3) populateStoresFromState looks at the route state and makes sure the stores have all the data they need to support that route state. 4) React renders the component tree. 5) The components read the current data from the appropriate Reflux store. 6) If the user uses a component to navigate to a new state, it writes to the URL, which kicks off 1) again.

So the components can read data, but writing data is taken care of by populateStoresFromState. The components only write to the URL, which causes populateStoresFromState to write the correct data to the stores.

With the getting only in your components and the setting only in populateStoresFromState, you shouldn't have a lot of duplicate code.

gpbl commented 9 years ago

@appsforartists awesome I think your solution fits very well, yet I'm not sure how should work populateStoresFromState. Could you provide an example?

appsforartists commented 9 years ago

@gpbl I'm not very familiar with Reflux, so some of the details might be wrong, but I imagine it'd be something like this:

var AllCategoriesStore   = require("./stores/AllCategoriesStore.js");
var CurrentCategoryStore = require("./stores/CurrentCategoryStore.js");
var CurrentProductStore  = require("./stores/CurrentProductStore.js");

var { 
  loadAllCategories,
  showCategory,
  showProduct
} = require("./actions.js");

function populateStoresFromState(
  routeState,
  cookies
) {
  var pendingPromises = [];

  // global data - we need this for every request
  pendingPromises.push(
    new Promise(
      (resolve, reject) => AllCategoriesStore.listen(resolve)
    )
  );

  loadAllCategories();

  // local data - depends on routeState
  if (routeState.params.hasOwnProperty("categoryKeyName")) {
    pendingPromises.push(
      new Promise(
        (resolve, reject) => CurrentCategoryStore.listen(resolve)
      )
    );

    showCategory(routeState.params.categoryKeyName);
  }

  if (routeState.params.hasOwnProperty("productKeyName")) {
    pendingPromises.push(
      new Promise(
        (resolve, reject) => CurrentProductStore.listen(resolve)
      )
    );

    showProduct(routeState.params.productKeyName);
  }

  return Promise.all(pendingPromises);
}

module.exports = populateStoresFromState;

Obviously, you could DRY out the promise code, but it's shown here more directly for explanation. You'd also need a way to handle errors.

gpbl commented 9 years ago

@appsforartists now it's clear also for me, thank you :+1:

There are two things I'd like to see better defined:

on the client, the populateStoresFromState must access the dehydratedStores to avoid requesting the same data again. This should be trivial using Reflux.hydrate, right?
on the server, how is the store constrained to the request's context? In your last code, wouldn't the same store listen to any requests? Said in another way: what happens when another request access the same store while the first is not yet populated? Is this the storeManager that @willembult was talking about? Because actually this is the point where I got lost :(

appsforartists commented 9 years ago

Making Reflux instantiable so you can have a new version for each request is definitely within the scope of this PR. The simplest way to do it (from an API point of view) would be to make Reflux a constructor that accepts the named parameters actions and stores:

var makeReflux = function () {
  return new Reflux(
    {
      "actions":  [
                    "loadAllCategories",
                    "showCategory",
                    "showProduct"
                  ],

      "stores":   {
                    "allCategories":    {
                                          "init":  function () {…}
                                        },

                    "currentCategory":  {
                                          "init":  function () {…}
                                        },
                  },
    }
  );
}

module.exports = makeReflux;

Then you would pass the result of that function into populateStores. makeReflux would run on each request, but only once on the client.

As for serialization/deserialization, that would happen at the Reflux level (in fact, probably in the constructor above). populateStores wouldn't really change. When its promise resolves, you'd serialize all your resulting stores, and you'd pass them into makeReflux on the client, where they'd be deserialized as the stores are initialized.

Damn, with all the code samples I've written for this ticket, I could just about mock out what the isomorphic Reflux API should look like.

dashed commented 9 years ago

Totally off topic, but I like to see how omniscientjs would be combined into an isomorphic architecture along with reflux.

I've been using omniscientjs with reflux only on the client-side.

gpbl commented 9 years ago

@appsforartists indeed :-) So, long story short, an isomorphic-friendly reflux would mean to make it instantiable. This is the reasoning behind Dispatchr, and the reason I landed to reflux, since I think its simplicity could win.

@Dashed thank you for reporting omniscientjs – it's very nice. i believe with reflux you would meet the same problems discussed here.

xavierelopez commented 9 years ago

@gpbl I agree that the simplicity and plug and play nature of reflux would be affected. Perhaps that's a good argument for providing this as a plugin or as an optional feature?

appsforartists commented 9 years ago

@xavierlopez Let's solve the API before we decide how to get there from here. Backwards compatibility is always a goal, so perhaps we can teach Reflux to use a default static instance of itself until a new one is instantiated by the user.

I think solving this well will require some architectural changes to Reflux's implementation, but that doesn't necessarily mean we need to break the interface.

gpbl commented 9 years ago

In the @appsforartists API mockup, reflux needs to know about stores and actions before being used. Dispatchr has in effect a registerStore method for that. An alternative for this would be introducing a context into which Store and Actions should run: this would require to pass it through the whole component tree, which leads to other complications. How could instead a plugin work, as @xavierelopez suggests?

xavierelopez commented 9 years ago

@appsforartists agreed.

@gpbl I didn't think of implementation details but if something like a reflux-isomorphic package existed, it'd contain a reflux instance maker, along with dehydration and rehydration utilities. I agree with @appsforartists though, maybe it's too early to consider that option since we're not clear on what would need to change yet.

gpbl commented 9 years ago

Here there's an other idea:

On the server-side, I see no reason that everyone can't share a single flux store. Each request to the store would also need the userid for whom you're fetching the information, and apps running client-side would get a 403 error when trying to access data for another user. Your flux store may act as a cache, or may query the database directly, but either way, by making store requests that include the user, you can now share a single store across the app again.

Could a reflux plugin help with sharing the same user id between stores?

appsforartists commented 9 years ago

@gpbl I saw that comment as well (subscribed to that thread).

That could be an option if you want to use Reflux on the server right this second, but I still think moving from globals to instances is a better solution long term. It's safer against accidentally leaking data across requests, and it supports running multiple React apps from a single server.

appsforartists commented 9 years ago

@spoike Haven't heard from you in a while. Is this an area you'll be working on, or does someone else need to take the reigns?

I'm happy to help out, but it seems like a solid understanding of the current Reflux internals would be really helpful - giving you first refusal.

appsforartists commented 9 years ago

I just looked into this a bit more, and I don't know that Reflux itself needs to be instantiable since stores and actions already are.

Rather than exporting single instances (module.exports = Reflux.create*), I think I'll export just the definitions and rely on a factory to instantiate them. I can run it for each request on the server, but only for the initial render on the client. If the same factory can be responsible for initializing the stores on the client, I may be able to solve this whole problem without touching a line of the Reflux internals. I'll keep you posted on what I find. If it works, we can talk about making a sugar for it, either in Reflux or in a separate module.

:smiley:

gpbl commented 9 years ago

@appsforartists I'm thrilled! :+1: please keep us updated!

spoike commented 9 years ago

@appsforartists I'm quiet because of RL stuff having priority and I currently only have time to triage the refluxjs issues for now. Currently bugs (:bug:) have higher priority now than new features. I'll jump into the discussion when time allows (like when I'm on vacation or something :santa:) and I'm always open for suggestions.

Regarding the internals of Reflux, basically it is currently just a bunch of stores and actions that are created with mixins. In my humble opinion the only state we need to worry about is the one that is in stores, which should depend on the context (on server-side is most definitely the session and whatever meta data you put in the request).

I'm all for mocks and pull requests! Please do create pull requests and link to this issue.

appsforartists commented 9 years ago

Completely understandable @spoike. Thanks for the work you've done so far!

I've started digging in now. Before, this morning, my experience with Reflux was about as deep as this thread - it's nice to be trying out actual code.

So far, the biggest pain point I feel is the lack of being able to "record" a series of actions and introspect which stores they trigger. This would be fantastic:

var triggeredActions = Reflux.recordActions(actions);

// app-specific code:
loadAllCategories();
loadCategory("bikes");

// generic code again:
var storeStateMap = {};

triggeredActions.getListeners().forEach(
  listener => storeStateMap[listener.name] = listener.state
);

Then, you'd pass storeStateMap to the client where it could be used to initialize the stores. Being able to record all the actions that have happened over a period of time and map them back to the stores they modified is nice, because it means you can solve serialization generically and all the app-specific code has to do is pass in a function that triggers the appropriate actions. The solution would then start recording, call the function, stop recording, serialize all the state, and pass it to the client.

I'm also thinking that supporting the this.state convention is a better solution than implementing hydrate/dehydrate. Serialization then becomes this simple:

var stateForClient = store.state;

store.state = stateFromServer;

appsforartists commented 9 years ago

I have successfully made my app wait for an async call to populate a store before returning the server-rendered result. It was a shit-ton of work (not all of it Reflux's fault - implementing something like this just involves touching a lot of things), so I still have a fair bit of cleanup and thinking to do, but I should have something exciting to share next week!

spoike commented 9 years ago

Cool. :+1:

geekyme commented 9 years ago

Ran into this issue as well as I was trying to make my react architecture isomorphic. I realized that at the moment Reflux is most useful on the client only because:

Components trigger actions that update store's state.
Store's state change trigger component updates.

On the server side, we don't need any of the above. Essentially all we need is a 'snapshot' of a store's state (or simply, page data) and send it down to the client. Probably the only way to do this right now is to duplicate XHR calls on the server side router, get the data and pass it as props down to React.renderToString.

Since we only need data on the server side, I was wondering if there can be a function on Reflux's stores that:

On client side, initiate stores & listeners, fire XHR requests and save client side state
On server side, fire XHR requests and return data to pass as props to React.renderToString

Since server side does not save any state, we would not need to worry about state leakage from one client to another.

reflux / refluxjs

Make reflux isomorphic friendly #144