Nested promises in server rendering

[jasonphillips]

I'm running into a problem with server rendering that may simply reflect a misunderstanding of the server capabilities of Resolver.renderToString.

My project uses react-router and has nested Resolver containers, so that /category/[x] might use a container to fetch a list of items under category x, and then /category/[x]/item/[y] would fetch a particular item in full detail, and so on with further nesting. All of this runs smoothly on the client.

When server rendering with Resolver.renderToString, it appears only to work through one level of Resolver containers; subsequent containers that create promises down the tree after the first one resolves will end up giving me a variant on this error:

Resolver is frozen for server rendering. 
ResolverContainer (#.0.0.0) should have already resolved "item". 

Is there a strategy that can be used for this scenario, forcing Resolver to wait for all promises and resolutions down the tree before freezing itself? Or should I be approaching it from a different angle?

[ericclemmons]

@jasonphillips This is what the resolver should be doing by design, but there may be slight variances between the projects I'm using it in & yours.

Would you be able to create a very simplistic example of this?

You can see in resolver.finish() that this is anticipated and tested (but apparently not enough!):

https://github.com/ericclemmons/react-resolver/blob/e039c24cf991c89b73dbcb0f4c60861d4c0a9a57/src/Resolver.js#L19-L29

If you have questions or need help getting started with a test case, I'll be glad to help out!

[ericclemmons]

i haven't heard back, so I guess I'll need to replicate this? Or at the very least, have a more complex server-side example...

[jasonphillips]

Apologies for the delay; hopefully I can put something together this weekend.

I threw in a quick fix in my project just to get through some tasks this week:

@@ -32,6 +32,10 @@ export default class Resolver {
     this.frozen = true;
   }

+  unfreeze() {
+    this.frozen = false;
+  }
+
   fulfillState(state, callback) {
     state.error = undefined;
     state.fulfilled = true;
@@ -106,11 +110,7 @@ export default class Resolver {
     }

     if (this.frozen) {
-      throw new ResolverError([
-        "Resolver is frozen for server rendering.",
-        `${container.constructor.displayName} (#${container.id}) should have already resolved`,
-        `"${asyncKeys.join("\", \"")}". (http://git.io/vvvkr)`,
-      ].join(" "));
+      this.unfreeze();
     }

     const promises = asyncKeys.map((prop) => {
@@ -168,17 +168,27 @@ export default class Resolver {
     return instance;
   }

+  static renderStringOnFinish(context, instance) {
+    return instance.finish().then(() => {
+      instance.freeze();
+
+      var stringOutput = React.renderToString(context);
+
+      if (!instance.frozen) {
+        return this.renderStringOnFinish(context, instance);
+      }
+
+      return stringOutput;
+    });
+  }
+
   static renderToString(element) {
     const resolver = new Resolver();
     const context = <Container resolver={resolver}>{element}</Container>;

     React.renderToString(context);

-    return resolver.finish().then(() => {
-      resolver.freeze();
-
-      return React.renderToString(context);
-    });
+    return this.renderStringOnFinish(context, resolver);
   }

The temporary fix was to make the static rendering part recursive, so that it unfreezes if a new promise is generated upon any subsequent render, looping until successfully frozen. That now works with any number of nested levels in my project. It might point towards a way of fixing it properly but, as you said, I need to find time for a reproducible test case first.

[ericclemmons]

This is interesting! The finish action is supposed to do precisely this. The only time this didn't occur was if a Promise was created outside of componentWillMount.

I definitely need to see why your scenario is differing from mine... is your code nothing but nested Resolver.createContainers?

[jasonphillips]

I've created some more complex tests for server rendering -- one with a simple set of nested Resolver containers, and one with containers where the child promise depends upon a property resolved in the parent -- and they are all running without error.

https://github.com/jasonphillips/react-resolver/commit/159c268097a2b16ddf8687f1099e6b48cb37401b

That implies my problem is introduced somewhere further into the complexity of my project, so I'll try to expand the tests from here.

That being said, I'm beginning to think that the problem I'm running into is at least partly caused in combination with react-router. I may go ahead and try to put in a couple tests for that as well. I realize that you have the example project successfully using the router (as is mine, except for the limit cases), and that it is not a requirement so much as an optional supported library; would you consider incorporating actual tests for router compatibility to be appropriate?

[ericclemmons]

@jasonphillips Sure, I see no problem with that. That's why I have the examples/stargazers portion of this project to ensure react-router behaves as expected.

Are there any cases where you're fetching data w/o promises, or in componentDidMount or componentWillMount?

[ericclemmons]

v2 resolves this. v1 has several issues we found in production.