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.

Sign up for GitHub

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

Jump to bottom

Update Remix to use route.lazy from React Router #7133

Merged
merged 9 commits into from
Aug 22, 2023
Merged

Update Remix to use route.lazy from React Router #7133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d532179
Update Remix to use route.lazy from React Router
brophdawg11 Aug 10, 2023
16c9364
Update to experimental RR version
brophdawg11 Aug 17, 2023
37f0947
Fix browser-entry test
brophdawg11 Aug 17, 2023
f176cf9
Add changeset
brophdawg11 Aug 17, 2023
4f1e0b8
todos
brophdawg11 Aug 17, 2023
12bb75c
Wire up forced HDR revalidation
brophdawg11 Aug 17, 2023
a543449
bye bye RemixRoute and RemixRouteError
brophdawg11 Aug 17, 2023
d29615b
remove unused imports
brophdawg11 Aug 17, 2023
67fa0f3
Merge branch 'dev' into brophdawg11/route-lazy
brophdawg11 Aug 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/sour-parrots-dream.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@remix-run/react": minor
"@remix-run/server-runtime": minor
"@remix-run/testing": minor
---

Update Remix to use React Router `route.lazy` for module loading
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ tsconfig.tsbuildinfo
/fixtures/test
/fixtures/my-remix-app
/fixtures/deno-app
/playwright-report
/integration/playwright-report
/test-results
/uploads

Expand Down
41 changes: 24 additions & 17 deletions packages/remix-react/browser.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,26 @@ if (import.meta && import.meta.hot) {
*/
export function RemixBrowser(_props: RemixBrowserProps): ReactElement {
if (!router) {
// Hard reload if the path we tried to load is not the current path.
// This is usually the result of 2 rapid back/forward clicks from an
// external site into a Remix app, where we initially start the load for
// one URL and while the JS chunks are loading a second forward click moves
// us to a new URL. Avoid comparing search params because of CDNs which
// can be configured to ignore certain params and only pathname is relevant
// towards determining the route matches.
let initialPathname = window.__remixContext.url;
let hydratedPathname = window.location.pathname;
if (initialPathname !== hydratedPathname) {
let errorMsg =
`Initial URL (${initialPathname}) does not match URL at time of hydration ` +
`(${hydratedPathname}), reloading page...`;
console.error(errorMsg);
window.location.reload();
// Get out of here so the reload can happen - don't create the router
// since it'll then kick off unnecessary route.lazy() loads
return <></>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this we get into I think the same hydration error loop as #1678. I hope to dig deeper into that later today/tomorrow so maybe it'll shed some light on why we need the short circuit, but once I move this up and return (and avoid any router initialization + lazy calls on the wrong route) the E2E test passes and all of my manual testing with rapid forward clicks into a Remix app to cause a mismatched URL at hydration time settle correctly on the final URL after the hard reload.

}

let routes = createClientRoutes(
window.__remixManifest.routes,
window.__remixRouteModules,
Expand All @@ -190,31 +210,18 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement {
},
});

// Hard reload if the path we tried to load is not the current path.
// This is usually the result of 2 rapid back/forward clicks from an
// external site into a Remix app, where we initially start the load for
// one URL and while the JS chunks are loading a second forward click moves
// us to a new URL. Avoid comparing search params because of CDNs which
// can be configured to ignore certain params and only pathname is relevant
// towards determining the route matches.
let initialPathname = window.__remixContext.url;
let hydratedPathname = window.location.pathname;
if (initialPathname !== hydratedPathname) {
let errorMsg =
`Initial URL (${initialPathname}) does not match URL at time of hydration ` +
`(${hydratedPathname}), reloading page...`;
console.error(errorMsg);
window.location.reload();
}

// Notify that the router is ready for HMR
if (hmrRouterReadyResolve) {
hmrRouterReadyResolve(router);
}
}

// This is due to the shit circuit return above which is an exceptional
// scenario which we can't hydrate anyway
// eslint-disable-next-line react-hooks/rules-of-hooks
let [location, setLocation] = React.useState(router.state.location);

// eslint-disable-next-line react-hooks/rules-of-hooks
React.useLayoutEffect(() => {
return router.subscribe((newState) => {
if (newState.location !== location) {
Expand Down
74 changes: 0 additions & 74 deletions packages/remix-react/components.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,21 @@ import {
Await as AwaitRR,
Link as RouterLink,
NavLink as RouterNavLink,
Outlet,
UNSAFE_DataRouterContext as DataRouterContext,
UNSAFE_DataRouterStateContext as DataRouterStateContext,
matchRoutes,
useAsyncError,
useActionData as useActionDataRR,
useLoaderData as useLoaderDataRR,
useRouteLoaderData as useRouteLoaderDataRR,
useMatches as useMatchesRR,
useLocation,
useNavigation,
useHref,
useRouteError,
} from "react-router-dom";
import type { SerializeFrom } from "@remix-run/server-runtime";

import type { AppData } from "./data";
import type { RemixContextObject } from "./entry";
import { RemixRootDefaultErrorBoundary } from "./errorBoundaries";
import invariant from "./invariant";
import {
getDataLinkHrefs,
Expand Down Expand Up @@ -83,55 +79,6 @@ function useRemixContext(): RemixContextObject {
return context;
}

////////////////////////////////////////////////////////////////////////////////
// RemixRoute

export function RemixRoute({ id }: { id: string }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

RemixRoute and RemixRouteError were sort of poor-mans implementations of Component/ErrorBoundary lazy loading. We could assign them to a route definition up front (without a route module available) knowing that they'd pull from the lazily loaded routeModulesCache at render-time. Now we can just assign Component/ErrorBoundary via lazy().

The only downside is we lose these error messages - so if we think they are worth keeping we can stick with this approach.

let { routeModules } = useRemixContext();

invariant(
routeModules,
"Cannot initialize 'routeModules'. This normally occurs when you have server code in your client modules.\n" +
"Check this link for more details:\nhttps://remix.run/pages/gotchas#server-code-in-client-bundles"
);

let { default: Component, ErrorBoundary } = routeModules[id];

// Default Component to Outlet if we expose boundary UI components
if (!Component && ErrorBoundary) {
Component = Outlet;
}

invariant(
Component,
`Route "${id}" has no component! Please go add a \`default\` export in the route module file.\n` +
"If you were trying to navigate or submit to a resource route, use `<a>` instead of `<Link>` or `<Form reloadDocument>`."
);

return <Component />;
}

export function RemixRouteError({ id }: { id: string }) {
let { routeModules } = useRemixContext();

// This checks prevent cryptic error messages such as: 'Cannot read properties of undefined (reading 'root')'
invariant(
routeModules,
"Cannot initialize 'routeModules'. This normally occurs when you have server code in your client modules.\n" +
"Check this link for more details:\nhttps://remix.run/pages/gotchas#server-code-in-client-bundles"
);

let error = useRouteError();
let { ErrorBoundary } = routeModules[id];

if (ErrorBoundary) {
return <ErrorBoundary />;
} else if (id === "root") {
return <RemixRootDefaultErrorBoundary error={error} />;
}

throw error;
}
////////////////////////////////////////////////////////////////////////////////
// Public API

Expand Down Expand Up @@ -1050,27 +997,6 @@ export interface RouteMatch {
handle: undefined | { [key: string]: any };
}

export function useMatches(): RouteMatch[] {
let { routeModules } = useRemixContext();
let matches = useMatchesRR();
return React.useMemo(
() =>
matches.map((match) => {
let remixMatch: RouteMatch = {
id: match.id,
pathname: match.pathname,
params: match.params,
data: match.data,
// Need to grab handle here since we don't have it at client-side route
// creation time
handle: routeModules[match.id].handle,
Comment on lines -1064 to -1066
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 longer need to grab handle from the routeModulesCache since it's populated via lazy()

};
return remixMatch;
}),
[matches, routeModules]
);
}

/**
* Returns the JSON parsed data from the current route's `loader`.
*
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-react/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export {
useHref,
useLocation,
useMatch,
useMatches,
useNavigate,
useNavigation,
useNavigationType,
Expand Down Expand Up @@ -68,7 +69,6 @@ export {
LiveReload,
useLoaderData,
useRouteLoaderData,
useMatches,
useActionData,
RemixContext as UNSAFE_RemixContext,
} from "./components";
Expand Down
4 changes: 2 additions & 2 deletions packages/remix-react/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
"typings": "dist/index.d.ts",
"module": "dist/esm/index.js",
"dependencies": {
"@remix-run/router": "0.0.0-experimental-ad6954b7",
"@remix-run/router": "0.0.0-experimental-4286521e",
"@remix-run/server-runtime": "1.19.3",
"react-router-dom": "0.0.0-experimental-ad6954b7"
"react-router-dom": "0.0.0-experimental-4286521e"
},
"devDependencies": {
"@testing-library/jest-dom": "^5.17.0",
Expand Down
Loading