Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RRR Step 2 - Server Rendering #4669

Merged
merged 8 commits into from Nov 28, 2022
Merged

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Nov 22, 2022

Initial work towards step 2 of React Router-ing Remix

  • Server rendering via unstable_createStaticRouter/StaticRouterProvider
  • No hydration handled yet, so only test on apps without <Scripts /> 😉
  • The duplicate copy of @remix-run/router is removed in favor of 1.0.4-pre.0

For branch integration testing, all SSR integration tests have been temporarily given .only and can be run with:

yarn test:integration action-test bug-report-test catch-boundary-data-test catch-boundary-test error-boundary-test form-data-test headers-test helpers/create-fixture path-mapping-test prefetch-test rendering-test splat-routes-test form-test resource-routes-test upload-test --project chromium

For local app testing:

  • Remove the <Scripts /> component from your local remix app since this branch only handles server-rendering
  • Install 1.8.0-pre.2 into your local remix app
  • Build this branch into your local remix app:
    • REMIX_LOCAL_BUILD_DIRECTORY=../path/to/your/app yarn build
  • Remove stale duped dependencies
    • rm -rf node_modules/@remix-run/server-runtime/node_modules
    • rm -rf node_modules/@remix-run/react/node_modules
    • rm -rf node_modules/react-router-dom/node_modules
    • rm -rf node_modules/react-router/node_modules
  • npm run dev

Look for ✅ Rendering RR 6.4 StaticRouterProvider in your server log to confirm that the new rendering path is applied

@changeset-bot
Copy link

changeset-bot bot commented Nov 22, 2022

⚠️ No Changeset found

Latest commit: 21f28d1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Base automatically changed from brophdawg11/rrr-bump-6.4.3 to feat/rrr-rendering November 22, 2022 19:58
@brophdawg11 brophdawg11 marked this pull request as ready for review November 28, 2022 23:47
@brophdawg11 brophdawg11 merged commit d13faa3 into feat/rrr-rendering Nov 28, 2022
@brophdawg11 brophdawg11 deleted the rrr-server-render branch November 28, 2022 23:49
brophdawg11 added a commit that referenced this pull request Dec 1, 2022
* First pass at SSR using react-router-dom@6.4
* useLoaderData/useActionData generics + useTransition/useFetcher types
* Remove duped router in favor of 1.0.4-pre.0
brophdawg11 added a commit that referenced this pull request Dec 2, 2022
* First pass at SSR using react-router-dom@6.4
* useLoaderData/useActionData generics + useTransition/useFetcher types
* Remove duped router in favor of 1.0.4-pre.0
brophdawg11 added a commit that referenced this pull request Dec 9, 2022
* First pass at SSR using react-router-dom@6.4
* useLoaderData/useActionData generics + useTransition/useFetcher types
* Remove duped router in favor of 1.0.4-pre.0
brophdawg11 added a commit that referenced this pull request Dec 14, 2022
* First pass at SSR using react-router-dom@6.4
* useLoaderData/useActionData generics + useTransition/useFetcher types
* Remove duped router in favor of 1.0.4-pre.0
Comment on lines -70 to -81
interface RemixEntryContextType {
manifest: AssetsManifest;
matches: BaseRouteMatch<ClientRoute>[];
routeData: RouteData;
actionData?: RouteData;
pendingLocation?: Location;
appState: AppState;
routeModules: RouteModules;
serverHandoffString?: string;
clientRoutes: ClientRoute[];
transitionManager: ReturnType<typeof createTransitionManager>;
future: FutureConfig;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪓 Majority of this is now handled by the router. A new RemixContext keeps the Remix-specific things: manifest, routeModules, serverHandoffString and future. <RemixServer> and <RemixBrowser> render <RemixContext.Provider> directly

</RemixCatchBoundary>
</RemixErrorBoundary>
</RemixEntryContext.Provider>
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buh bye <RemixEntry>!

get data() {
console.error("You cannot `useLoaderData` in an error boundary.");
return undefined;
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this functionality is probably still missing - should get this added to the router

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bring this over to router - we will have a larger conversation about this in the future

element = (
if (!isRouteErrorResponse(error) && ErrorBoundary) {
return (
// TODO: Handle error type?
<RemixErrorBoundary
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment we just proxy through to the current RemixErrorBoundary/RemixCatchBounary componnts, but when we add the v2_errorBoundary flag this function will hopefully just short circuit to return <RouteModuleErrorBoundary />

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post-release: Dig into why tests fail if we remve RemixErrorBoundary in favor of <ErrorBoundary error={error}>

Comment on lines -551 to +337
let { matches, routeModules, manifest } = useRemixEntryContext();
let { manifest, routeModules } = useRemixContext();
let { matches } = useDataRouterStateContext();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most changes to existing/preserved hooks are just grabbing stuff from the new contexts

*
* @see https://remix.run/api/remix#form
*/
let Form = React.forwardRef<HTMLFormElement, FormProps>((props, ref) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Form, useFormAction, useSubmit, etc. are now re-exported from react-router-dom

}),
[matches, routeData, routeModules]
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually comes back since RR doesn't know about handle at route definition time so needs to populate from the routeModulesCache

/**
* Returns the JSON parsed data from the current route's `loader`.
*
* @see https://remix.run/api/remix#useloaderdata
*/
export function useLoaderData<T = AppData>(): SerializeFrom<T> {
return useRemixRouteContext().data;
return useLoaderDataRR() as SerializeFrom<T>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the current solution until we get proper generics in RR - then we can re-export directly. It felt like that was a whole undertaking unto itself though :)

import invariant from "./invariant";
import type { Submission } from "./transition";

export type AppData = any;

export type FormMethod = "get" | "post" | "put" | "patch" | "delete";
export type FormMethod = FormMethodRR;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something we need to do a pass on still - probably have lots of types that can either be deleted or just re-export something from RR to keep type-API compatibility

@@ -135,6 +134,7 @@ export function RemixCatchBoundary({
children,
}: RemixCatchBoundaryProps) {
if (catchVal) {
// TODO: Add Status/Data generics to ErrorResponse?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still outstanding

// Note: we don't need loader/action/shouldRevalidate on these routes
// since they're for a static render
handle: routeModules[route.id].handle,
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server-render-time routes don't have loaders/actions since we already fetched the data!

context={context.staticHandlerContext}
hydrate={false}
/>
</RemixContext.Provider>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👨‍🍳 💋

////////////////////////////////////////////////////////////////////////////////
//#region Types and Utils
////////////////////////////////////////////////////////////////////////////////
import type { Location, NavigationType as Action } from "react-router-dom";

export interface CatchData<T = any> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file still exists just for typings and IDLE_TRANSITION etc.

manifest: build.assets,
routeModules: createEntryRouteModules(build.routes),
serverHandoffString: createServerHandoffString(serverHandoff),
staticHandlerContext: context,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more appState - just work directly with the @remix-run/router staticHandlerContext now

);

// Restructure context.errors to the right Catch/Error Boundary
differentiateCatchVersusErrorBoundaries(build, context);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back-compat for bubbling error/catch correctly since the router doesn't have a differentiation

brophdawg11 added a commit that referenced this pull request Dec 16, 2022
* First pass at SSR using react-router-dom@6.4
* useLoaderData/useActionData generics + useTransition/useFetcher types
* Remove duped router in favor of 1.0.4-pre.0
brophdawg11 added a commit that referenced this pull request Dec 19, 2022
* Bump remix to react-router-dom@6.4.4 (#4668)
* RRR Step 2 - Server Rendering (#4669)
* RRR Step 3 - Hydration (#4703)
* RRR Rendering Follow Ups (#4885)
* ScrollRestoration on RR 6.4 (#4844)

Co-authored-by: Jacob Ebey <jacob.ebey@live.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: 🚀 Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants