Suggestion for handling isomorphism

[Satyam]

This is a suggestion, comments are appreciated.

I forked this repository and made some changes to Alt in my own fork plus an example to see it at work.

I have not added tests for my changes but I made sure all current tests pass, thus, my changes are backward compatible.

One of the changes, though, might be a good idea for Alt as it is, isomorphism or not. Async action handlers, instead of returning false or calling preventDefault to signal it is not ready yet, and then emitChange when done, can simply return a Promise. This can be seen at work here.

This feature is handled here where, instead of checking whether the action handler returned false or not, it first checks if it is a Promise and deals with it.

As a non-Promise (ES5) alternative, I meant to provide an extra argument to the action handler, a callback to be called when the async operation is over. If the argument count is 3, the dispatcher would assume the handler is asynchronous and wait for the callback. However, there is some test somewhere that checks the number of arguments on action handlers and rejects handlers with more than one argument. As a matter of fact, it doesn't allow you to receive the second action argument which is already being provided.

What I didn't like about Iso is that it had no means to know when the stores are stable. Thus, it had to make a snapshot lookalike and bootstrap it before getting to the server rendering part. I wanted to avoid that, but since the loading is async, the problem is to know when they are all ready. That is why I changed the dispatcher of the action handlers so that I can keep a count of the actions pending response in a way as transparent to Alt as possible. As a matter of fact, the stores are filled in response to the same actions as in the client and, in the end, I do call alt.takeSnapshot.

Moreover, I do this based not on the URL of the page requested but on the routes resolved by the React-Router, thus the need to maintain a sort of separate kind of routing table to load the stores depending on the full url is not needed.

I would recommend that route handling components have a static property listing the actions that initialize their stores:

UserStore.initActions = ['getUser`]

it would be easy to read those once the router finds the proper routes:

app.use(function (req, res, next) {
  Router.run(routes, req.path, function (Handler, state) {
    state.routes.forEach(function (route) {
      const acts = route.handler.initActions
      if (acts) acts.forEach(act => actions[act]())
    })
    alt.whenStable(function (dataEl) {
      res.render('layout', {
        html: React.renderToString(React.createElement(Handler, {params: state.params, query: state.query})) + dataEl
      })
    })
  })
})

I made two separate commits, one with the changes to alt and one with an example on how to use it. Both have in-line comments on each of the main features.

[taion]

How does your example deal with multiple simultaneous requests? It doesn't look like you have anything there to keep your stores from getting clobbered by overlapping requests. You need to either use instances or do some clever book-keeping with data-source-like abstractions (see #393 or #400) to avoid issues there.

[goatslacker]

I created a Promiseable dispatcher here: #234

it needs tests and stuff before I can merge it in.

[Satyam]

@taion One way I handle this is by storing an id of each action called on each store and preventing another call if the id matches. The id is made of the store displayName and the action as seen here. I am in doubt on whether to include a representation of the payload into that signature which is why, somewhat unwisely, I check that on the store. One of my fears in doing this is that the payload might contain too much ancillary information not relevant for the purpose of identifying unique requests, which might vary without really implying an actual separate request, so I left that check to the store. I don't like it though. Nor do I know if this covers all cases.

[Satyam]

@goatslacker good! I really didn't look for that, sorry.

My original need was to have a means to keep a count of pending requests on async action handlers. The existing mechanism didn't allowed me to detect when the action handling was done. Initially, I thought about providing a callback as an extra argument to the action handler, which I still have there. Then I thought Promises was an even better alternative so I provided that as well, but since it seemed trivial to patch that and not central to what I meant to do I didn't do much research into it, thus missed your patch.

Anyway, I've started using Alt about a month ago so I'm not that much into everything that is going on. I just wanted to solve my problem and thought I might as well share it.

[goatslacker]

Closing in favor of #393 or #400 or #234 which will all handle rendering whenever the store is ready.