Skip to content

Commit

Permalink
fix: short circuit links and meta for routes that are not rendered du…
Browse files Browse the repository at this point in the history
…e to errors (#6107)
  • Loading branch information
jacob-ebey committed Apr 25, 2023
1 parent 6e66529 commit 2f008d0
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/link-meta-short-circuit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/react": patch
---

short circuit links and meta for routes that are not rendered due to errors
60 changes: 60 additions & 0 deletions integration/link-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ test.describe("route module link export", () => {
<li>
<Link to="/resources">Resource routes</Link>
</li>
<li>
<Link to="/parent/child">Errored child route</Link>
</li>
</ul>
</nav>
</div>
Expand Down Expand Up @@ -471,6 +474,42 @@ test.describe("route module link export", () => {
}
`,

"app/routes/parent.jsx": js`
import { Outlet } from "@remix-run/react";
export function links() {
return [
{ "data-test-id": "red" },
];
}
export default function Component() {
return <div data-test-id="/parent"><Outlet /></div>;
}
export function ErrorBoundary() {
return <h1 data-test-id="/parent:error-boundary">Error Boundary</h1>;
}
`,

"app/routes/parent.child.jsx": js`
import { Outlet } from "@remix-run/react";
export function loader() {
throw new Response(null, { status: 404 });
}
export function links() {
return [
{ "data-test-id": "blue" },
];
}
export default function Component() {
return <div data-test-id="/parent"><Outlet /></div>;
}
`,
},
});
appFixture = await createAppFixture(fixture);
Expand Down Expand Up @@ -511,6 +550,17 @@ test.describe("route module link export", () => {
expect(stylesheetResponses.length).toEqual(1);
});

test("does not render errored child route links", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/", true);
await page.click('a[href="/parent/child"]');
await page.waitForSelector('[data-test-id="/parent:error-boundary"]');
await page.waitForSelector('[data-test-id="red"]', { state: "attached" });
await page.waitForSelector('[data-test-id="blue"]', {
state: "detached",
});
});

test.describe("no js", () => {
test.use({ javaScriptEnabled: false });

Expand All @@ -534,6 +584,16 @@ test.describe("route module link export", () => {
let locator = page.locator("link[rel=preload][as=image]");
expect(await locator.getAttribute("imagesizes")).toBe("100vw");
});

test("does not render errored child route links", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/parent/child");
await page.waitForSelector('[data-test-id="/parent:error-boundary"]');
await page.waitForSelector('[data-test-id="red"]', { state: "attached" });
await page.waitForSelector('[data-test-id="blue"]', {
state: "detached",
});
});
});

test.describe("script imports", () => {
Expand Down
35 changes: 32 additions & 3 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,14 @@ let fetcherSubmissionWarning =
*/
export function Links() {
let { manifest, routeModules } = useRemixContext();
let { matches } = useDataRouterStateContext();
let { errors, matches: routerMatches } = useDataRouterStateContext();

let matches = errors
? routerMatches.slice(
0,
routerMatches.findIndex((m) => errors![m.route.id]) + 1
)
: routerMatches;

let links = React.useMemo(
() => getLinksForMatches(matches, routeModules, manifest),
Expand Down Expand Up @@ -578,9 +585,20 @@ function PrefetchPageLinksImpl({
*/
function V1Meta() {
let { routeModules } = useRemixContext();
let { matches, loaderData } = useDataRouterStateContext();
let {
errors,
matches: routerMatches,
loaderData,
} = useDataRouterStateContext();
let location = useLocation();

let matches = errors
? routerMatches.slice(
0,
routerMatches.findIndex((m) => errors![m.route.id]) + 1
)
: routerMatches;

let meta: V1_HtmlMetaDescriptor = {};
let parentsData: { [routeId: string]: AppData } = {};

Expand Down Expand Up @@ -676,9 +694,20 @@ function V1Meta() {

function V2Meta() {
let { routeModules } = useRemixContext();
let { matches: _matches, loaderData } = useDataRouterStateContext();
let {
errors,
matches: routerMatches,
loaderData,
} = useDataRouterStateContext();
let location = useLocation();

let _matches = errors
? routerMatches.slice(
0,
routerMatches.findIndex((m) => errors![m.route.id]) + 1
)
: routerMatches;

let meta: V2_MetaDescriptor[] = [];
let leafMeta: V2_MetaDescriptor[] | null = null;
let matches: V2_MetaMatches = [];
Expand Down

0 comments on commit 2f008d0

Please sign in to comment.