setImmediate forces Render to be async

[kirbdee]

I'm observing the issue where the time between hydration and rendering/mounting can increase / blocked due to setImmediate forcing rendering to be async and putting it back into the "task queue"

My observations: https://github.com/kirbdee/FluxibleSample/blob/master/README.md
Code: https://github.com/kirbdee/FluxibleSample

[mridgway]

I'm curious why removing setImmediate actually fixes the issue since .then's callback is also guaranteed to be async and should push the render back as well. The ordering of these methods could be browser specific which is something look into.

WRT the mention of handling errors, the setImmediate is there so that handles in the callback function do not get caught by an internal Promise within Fluxible.

app.rehydrate(state, (err, context) => {
   // err can be handled here
  if (err) {
      handleError(err);
      return;
  }

  myUndefinedFunction() // <-- throws an error, caught by internal Fluxible promise silently
});

This is pretty critical in my opinion, because it seems like something went wrong with Fluxible even though there was an error in the user's code. Unless we can solve this, I don't think we'd be willing to remove the setImmediate.

I'm curious about your example you mention that the second JS file is critical, but you want it to be loaded after the initial execution of the first file which seems contradictory to me. I would have suggested async as well, so I'm not sure I fully understand your reasoning there. Maybe a lazy load using require.ensure would be easier to reason about: you would be able to get the initial render out faster, call require.ensure so that the subsequent code that needs the second file is blocked by it being loaded.

[kirbdee]

Regarding the error handling, I see! but in Promise(we're using es6-promise right?) couldn't we do this:

app.rehydrate(state, (err, context) => {
   // err can be handled here
  if (err) {
      handleError(err);
      return;
  }

  myUndefinedFunction() // <-- throws an error, caught by internal Fluxible promise silently
}).catch(function(error) {
  throw error //<-- thrown back up
});

or internally

 if (callback) {
        rehydratePromise
            .then(function (contextValue) {
                callback(null, contextValue);
            }, function (err) {
                callback(err);
            }).catch(function(err) {
               throw error
            });
    }

per http://www.html5rocks.com/en/tutorials/es6/promises/#toc-error-handling (which was referred to from https://www.npmjs.com/package/es6-promise)

[mridgway]

Yeah, that's totally possible, but creates confusion since you now have to deal with both Promises and callbacks. I really hate having to support both >.<

[kirbdee]

@mridgway added another example internally instead, to be "hidden" from the user

[kirbdee]

setImmediate just seems so out of place to me, just to handle errors