ephem / react-lightyear Goto Github PK

So far Lightyear has mainly been tested in different smaller contexts and not a larger application. Tests do give good confidence it is working as expected, but some bugs only manifest in more complex environments, so it would be nice to try it out in a larger application.

I intend to do this with the application I build at work, but it would be very nice to have more data points! Since very few, if any, larger projects are using Suspense for data fetching together with a custom SSR-setup today, the easiest way to help out by testing this is probably if you have a custom SSR-setup with traditional data-fetching (Next.js sadly doesn't support custom renderers like this one).

The thing that differs most from the official server renderer is that when a suspend happens, the rest of that subtree will be rendered by a "child renderer" when the data comes back. Most bugs will probably stem from this fact. A good way to artificially stress-test Lightyear in a larger application would therefor be to fetch data via Suspense in a single top level-isch component, causing most of the application to be rendered by a child renderer. A partial refactor to fetch the real application-data with Suspense would also be very helpful of course, but a much larger time investment (unless you already use react-apollo-hooks, in which case it could be feasible).

Looking at the examples should give you a hint of how to go about this, but I would also be extremely happy to help out in any way I can!

Some suggestion:

• CHANGELOG.md – Start out by describing what changes have been made from the base React project, what the plan is for possibly upstreaming those changes (like a PR/Issue link) and then from there, for each version, just note the differences from the version before
• CONTRIBUTING.md– should probably highlight that everything not specific to the changes you have made should be filed against the upstream repo and not here. Only things related to your specific alteration should be reported here (I assume 😉)
• .github/ISSUE_TEMPLATE.md+ .github/PULL_REQUEST_TEMPLATE.md – evaluate whether these should also be updated

Currently Lightyear is versioned as semver with the following dependencies:

{
  "version": "0.1.0",
  "peerDependencies": {
    "react": "^16.0.0"
  },
  "optionalDependencies": {
    "react-dom": "16.8.6"
  }
}

It would be nice if Lightyear versions aligned with the React-versions (just as react and react-dom are aligned), however, I see two potential issues I'm not sure about:

• How to release bug fixes for Lightyear while retaining alignment with React versions? (Or for that matter majors or minors if functionality like fallbacks are added?)
• Lightyear touches a part of the React-codebase that is not updated very often (right now). Many releases might just be bumps to align with React. Not much of an issue, just an observation. :)

One idea could be to sync major and minors, but retain patch-versions for bumps to Lightyear itself?

Would love feedback on this!

Right now this fork has no CI or automation like release-scripts etc at all. I'd like to improve this to have more confidence in the development and release process.

• Add CI for PRs and lightyear-branch
• Add release automation

The new functionality (located mainly at the bottom of ReactPartialRenderer) was written without Flow-types since this started as an exploration and I wasn't comfortable moving fast with Flow. Now that things are more stable it would be nice to add types.

Feel free to help out if you are comfortable with Flow and want to explore this project, if anyone wants to tackle this I would love to help out. I will get around to this myself eventually, but it's not at the top of my list.

I'd like to improve the documentation of this project in a bunch of ways.

• Better Readme
• Better Readme in the examples, highlighting and detailing the main points
• Contribution guide

There are probably more things to improve, I'd love feedback and/or help with this. :)

As was explained in #32, hydration of <Suspense> in React is supposed to only work with blocking or concurrent roots, not with the "legacy" ReactDOM.render. That this has worked up until now has been unintentional and the v16.11.0 release broke this for good.

The plan moving forward is mainly to write docs to explain this better. Lightyear will move to "only" be compatible with the experimental React release-channel, which matches well with the experimental nature of Lightyear. I am not sure yet how this will affect versioning of Lightyear, or how peerDependencies will work with React, since the experimental release channel does not follow semver, feedback on this is very welcome!

A known workaround to get Lightyear to work with the "legacy" rendering mode is to have a <CustomSuspense>-component that renders a <Suspense> boundary on the server, a <Fragment> on the client which gets switched back to <Suspense> in an effect. This probably has a bunch of bad implications though. There might be other workarounds as well, I am not sure yet if I will document any of these or not.

• Figure out and decide on good strategy for versioning
• Update Readme for experimental release channel, that is, blocking/concurrent mode
• Update examples to concurrent mode and experimental release channel

First of all, thanks for making this easy to test Suspense in SSR. I've tried running a big application with it and hit some edge cases.
It looks like that context can be a null object, so the methods expecting on it to be an object throw. Didn't have time to investigate why context was null in the first place (my guess is maybe a bug in popProvider) but fixed it by checking for null where context was expected:

function ReactDOMServerRendererAsync(children, makeStaticMarkup, renderContext) {
      // ...
      for (var i = 0; i < _this.contextStack.length; i += 1) {
        if (!_this.contextStack[i]) { // <- contextStack[I] can be null
          _this.contextStack[i] = {};
        }
        _this.contextStack[i][_this.threadID] = renderContext.contextValues[i];
      }
      // ...
}

And also

ReactDOMServerRendererAsync.prototype.getClonedRenderContext = function getClonedRenderContext() {
    var _this2 = this;

    var contextValues = this.contextStack.map(function (context) {
      return context && context[_this2.threadID]; // <- here context can be null
    });
    // ...
}

It looks like react-dom streamer splits the output into chunks of 14Kb. However that chunking is taken into account even when Suspense is being used, which means that the output might be truncated way before the Suspense boundary and the user might see a section of the page half rendered.

I tried to track down when this happens and it looks like when ReactDOMServerRendererAsync.readAsync is called, if it finds a REACT_SUSPENSE_TYPE frame then it sets the Suspense promise as second index of the current queue, later picked up by flattenAndResolveQueue which awaits for it.
The problem is that the queue might contain other html data that is hold until the Suspense promise is resolved, potentially leaving the user with a half streamed component.
We should flush all the queue html before awaiting for the suspense promise (quickly tried but couldn't make it work).

Being a fork, Lightyear will have to sync upstream changes from the React repo when new React-versions are released. Most of the code is separated in a way that should avoid large merge conflicts, and there also isn't currently that much activity in the affected parts of the React repo.

Even though I don't foresee any problems, before marking this project "stable" (#12) I would like to evaluate how easy it is to keep this fork in sync with React by syncing the upstream changes once. This could either be done with a real React-release if it happens soon, or just do an experiment with syncing towards master.

After preparing a merge and release for v16.10.0 I discovered a problem during testing, namely that hydration is broken when using Lightyear.

React v16.10.0 has ongoing work for Partial hydration which v16.9.0 did not have. From what I can tell the behaviour when React encounters a <Suspense> boundary during hydration has changed.

Error is: "Expected to have a hydrated suspense instance." which comes from prepareToHydrateHostSuspenseInstance.

Sadly, I don't think this can be fixed properly in Lightyear itself (but I'm 100% sure yet), in that case I see two possible ways forward (neither of which are verified to work):

• Needing to use a custom-built react-dom together with Lightyear, with the enableSuspenseServerRenderer feature flag turned on (and/or some other tweak)
• App code only rendering <Suspense> on the server, outputting a fragment on the client. This could be changed back to a <Suspense>-component after hydration.

When rendering the following component to string:

const Component = <>{'a'} {'b'}</>;

the expected output is "a<!-- --> <!-- -->b". This is currently broken in any component that is a child or deep child of a component that did a suspend and instead returns "a b" which causes a mismatch on client hydration.

I plan to fix this very soon.

Since this is a fork that circles around an unstable React-api and exists solely to fill in the SSR-gap for that until an official solution exists, it will never be "stable" in the traditional sense.

However, at some point I would like to indicate that I'm confident that this is stable enough for use if you know the tradeoffs and risks you are taking. This issue will track that progress.

• Benchmark performance (#13)
• Try out Lightyear in a larger app (#14)
• Make sure syncing upstream changes is feasible (#15)
• Improve project infra (#16)
• Improve documentation (#17)

Being considered "stable" will be marked by adopting the versioning strategy proposed in #1, that is, following React major and minor, while keeping patch separate.

Any chance of an example project with minimal setup?

A main motivation for this project is that a single asynchronous render pass should be faster than doing a separate prepass to populate data-caches, but is it?

I'd like to have a couple of decent benchmarks and compare Lightyear, react-ssr-prepass, the getDataFromTree-function from Apollo and possibly others under a few different conditions. Size of tree and nested data fetching or not are two conditions that come to mind.