zth/rescript-relay-router

Remove `Suspense` boundary in `RelayRouter.RouteRenderer` component

Kingdutch opened this issue · 1 comments

#Problem
The RelayRouter RouteRenderer component currently renders a suspense boundary. This prevents applications from building up a minimum amount of loaded data before sending a UI to the client.

We rely on the onShellReady to start sending data to the client. React 18 considers the "shell" to be "anything that's not within a suspense boundary". This provides the UI engineer power to determine how fleshed out the app-shell should be and define a data-minimum.

Because the RouteRenderer renders a suspense boundary, no data loading (leveraging the router) can happen to produce an app shell. This prevents e.g. rendering at least a full header on the server side while allowing the rest of the content to stream in.

Proposal

Remove the renderFallback argument to RelayRouter.RouteRenderer and do not render a Suspense boundary in the router itself. This allows applications to define what data they want loaded before responding to the client.

Users who want to send something to the client without waiting for any data can still achieve this by defining the suspense boundary around the router themselves.

Replacing:

          <RelayRouter.RouteRenderer
            // Fallback renders whenever suspense bubbles all the way up here, to the router
            renderFallback={_ => React.string("Fallback...")}

            // This renders all the time, and when there's a pending navigation (pending via React concurrent mode), pending will be `true`
            renderPending={pending => pending ? React.string("Pending....") : React.null}
          />

with

        <React.Suspense fallback={React.string("Fallback...")}>
          <RelayRouter.RouteRenderer
            // This renders all the time, and when there's a pending navigation (pending via React concurrent mode), pending will be `true`
            renderPending={pending => pending ? React.string("Pending....") : React.null}
          />
        </React.Suspense>
zth commented

Yes, very good point! Would you like to PR that change?