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

fix(remix-react): avoid duplicate loader calls when using prefetch-intent #2938

Merged
merged 5 commits into from
May 16, 2022
Merged
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/api/remix.md
Original file line number Diff line number Diff line change
@@ -125,7 +125,7 @@ In our effort to remove all loading states from your UI, `Link` can automaticall
```

- **"none"** - Default behavior. This will prevent any prefetching from happening. This is recommended when linking to pages that require a user session that the browser won't be able to prefetch anyway.
- **"intent"** - Recommended if you want to prefetch. Fetches when Remix thinks the user intends to visit the link. Right now the behavior is simple: if they hover or focus the link it will prefetch the resources. In the future we hope to make this even smarter. Links with large click areas/padding get a bit of a head start.
- **"intent"** - Recommended if you want to prefetch. Fetches when Remix thinks the user intends to visit the link. Right now the behavior is simple: if they hover or focus the link it will prefetch the resources. In the future we hope to make this even smarter. Links with large click areas/padding get a bit of a head start. It is worth noting that when using `prefetch="intent"`, `<link rel="prefetch">` elements will be inserted on hover/focus and removed if the `<Link>` loses hover/focus. Without proper `cache-control` headers on your loaders this could result in repeated prefetch loads if a user continually hovers on and off a link.
- **"render"** - Fetches when the link is rendered.

<docs-error>You may need to use the <code>:last-of-type</code> selector instead of <code>:last-child</code> when styling child elements inside of your links</docs-error>
32 changes: 27 additions & 5 deletions integration/prefetch-test.ts
Original file line number Diff line number Diff line change
@@ -52,7 +52,7 @@ function fixtureFactory(mode: RemixLinkProps["prefetch"]) {

"app/routes/index.jsx": js`
export default function() {
return <h2>Index</h2>;
return <h2 className="index">Index</h2>;
}
`,

@@ -61,13 +61,13 @@ function fixtureFactory(mode: RemixLinkProps["prefetch"]) {
return { message: 'data from the loader' };
}
export default function() {
return <h2>With Loader</h2>;
return <h2 className="with-loader">With Loader</h2>;
}
`,

"app/routes/without-loader.jsx": js`
export default function() {
return <h2>Without Loader</h2>;
return <h2 className="without-loader">Without Loader</h2>;
}
`,
},
@@ -185,7 +185,29 @@ test.describe("prefetch=intent (hover)", () => {
"#nav link[rel='modulepreload'][href^='/build/routes/without-loader-']",
{ state: "attached" }
);
expect(await page.locator("#nav link").count()).toBe(3);
expect(await page.locator("#nav link").count()).toBe(1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryanflorence Now that we setShouldPrefetch(false) in cancelIntent these tests are invalid since the link tags remove - just confirming that's expected 👍

});

test("removes prefetch tags after navigating to/from the page", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");

// Links added on hover
await page.hover("a[href='/with-loader']");
await page.waitForSelector("#nav link", { state: "attached" });
expect(await page.locator("#nav link").count()).toBe(2);

// Links removed upon navigating to the page
await page.click("a[href='/with-loader']");
await page.waitForSelector("h2.with-loader", { state: "attached" });
expect(await page.locator("#nav link").count()).toBe(0);

// Links stay removed upon navigating away from the page
await page.click("a[href='/without-loader']");
await page.waitForSelector("h2.without-loader", { state: "attached" });
expect(await page.locator("#nav link").count()).toBe(0);
});
});

@@ -237,6 +259,6 @@ test.describe("prefetch=intent (focus)", () => {
"#nav link[rel='modulepreload'][href^='/build/routes/without-loader-']",
{ state: "attached" }
);
expect(await page.locator("#nav link").count()).toBe(3);
expect(await page.locator("#nav link").count()).toBe(1);
});
});
1 change: 1 addition & 0 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
@@ -420,6 +420,7 @@ function usePrefetchBehavior(
let cancelIntent = () => {
if (prefetch === "intent") {
setMaybePrefetch(false);
setShouldPrefetch(false);
}
brophdawg11 marked this conversation as resolved.
Show resolved Hide resolved
};