Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Use sapper:prefetch and sapper:external instead of rel attribute #1427

Closed
wants to merge 3 commits into from
Closed

Use sapper:prefetch and sapper:external instead of rel attribute #1427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e2efa3
change rel= to sapper: for prefetch and external
arekbartnik Aug 22, 2020
3e22f0d
Merge branch 'master' into rel_prefetch_external
arekbartnik Sep 26, 2020
e3d667c
changes after merge
arekbartnik Sep 26, 2020
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 runtime/src/app/prefetch/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function get_prefetched(target: Target): Promise<HydratedTarget> {
function trigger_prefetch(event: MouseEvent | TouchEvent) {
const a: HTMLAnchorElement = <HTMLAnchorElement>find_anchor(<Node>event.target);

if (a && a.rel === 'prefetch') {
if (a && a.hasAttribute('sapper:prefetch')) {
prefetch(a.href);
}
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/src/app/router/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ function handle_click(event: MouseEvent) {

// Ignore if tag has
// 1. 'download' attribute
// 2. rel='external' attribute
if (a.hasAttribute('download') || a.getAttribute('rel') === 'external') return;
// 2. 'sapper:external' attribute
if (a.hasAttribute('download') || a.hasAttribute('sapper:external')) return;

// Ignore if <a> has a target
if (svg ? (<SVGAElement>a).target.baseVal : a.target) return;
Expand Down
2 changes: 1 addition & 1 deletion site/content/docs/03-client-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const saveItem = () => {

* `href` — the page to prefetch

Programmatically prefetches the given page, which means a) ensuring that the code for the page is loaded, and b) calling the page's `preload` method with the appropriate options. This is the same behaviour that Sapper triggers when the user taps or mouses over an `<a>` element with [rel=prefetch](docs#rel_prefetch).
Programmatically prefetches the given page, which means a) ensuring that the code for the page is loaded, and b) calling the page's `preload` method with the appropriate options. This is the same behaviour that Sapper triggers when the user taps or mouses over an `<a>` element with [sapper:prefetch](docs#sapper_prefetch) attribute.

Returns a `Promise` that resolves when the prefetch is complete.

Expand Down
14 changes: 6 additions & 8 deletions site/content/docs/08-link-options.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,30 @@
title: Link options
---

### rel=prefetch
### sapper:prefetch

Sapper uses code splitting to break your app into small chunks (one per route), ensuring fast startup times.

For *dynamic* routes, such as our `src/routes/blog/[slug].svelte` example, that's not enough. In order to render the blog post, we need to fetch the data for it, and we can't do that until we know what `slug` is. In the worst case, that could cause lag as the browser waits for the data to come back from the server.

We can mitigate that by *prefetching* the data. Adding a `rel=prefetch` attribute to a link...
We can mitigate that by *prefetching* the data. Adding a `sapper:prefetch` attribute to a link...

```html
<a rel=prefetch href='blog/what-is-sapper'>What is Sapper?</a>
<a href='blog/what-is-sapper' sapper:prefetch>What is Sapper?</a>
```

...will cause Sapper to run the page's `preload` function as soon as the user hovers over the link (on a desktop) or touches it (on mobile), rather than waiting for the `click` event to trigger navigation. Typically, this buys us an extra couple of hundred milliseconds, which is the difference between a user interface that feels laggy, and one that feels snappy.

> `rel=prefetch` is a Sapper idiom, not a standard attribute for `<a>` elements

<!-- TODO add a function to prefetch programmatically -->

### rel=external
Copy link
Member

Choose a reason for hiding this comment

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

I'm not as sure that we should change this one. We're using rel=external as intended

Copy link
Contributor Author

@arekbartnik arekbartnik Aug 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I've recently needed to navigate to a site in same-origin which wasn't a part of a Sapper app. Without rel=external it's not possible.
My argumentation is that application actions should not be bound to a existing attributes (in long term this always creates problems - like with rel=prefetch).

Copy link
Member

Choose a reason for hiding this comment

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

I've recently needed to navigate to a site in same-origin which wasn't a part of a Sapper app. Without rel=external it's not possible.

I would think we should just make the router fall through and treat a link as a normal link in the case that it can't find a corresponding route

Copy link
Member

Choose a reason for hiding this comment

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

I'm confused about what specific case has an issue with treating rel=external in this way. I believe Sapper's behavior for the click interception is based on that of TJ's Page.js. In any case, IIRC Sapper already does bail on the interception if the link isn't to something that matches any route.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Conduitry my example:

  • the main page is an Sapper app with link to articles at example.com
  • articles aren't part of the Sapper app but are availble at example.com/category/article-title

Maybe both options should be available?

Copy link
Member

Choose a reason for hiding this comment

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

Tries to load it and then what happens? You get a 404? What do you mean that use use [...slug].svelte?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In routes I have [...slug].svelte to map all pathnames. So Sapper thinks there is a route for an article and after click loads template from [...slug].svelte with an error.
Then after page refresh VIP directs browser to a different app because articles are served by this app and not by Sapper.

I admit - it's a highly specific case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I should reduce this PR to a problem from #1401 and then open new issue about potential changes to rel=external?

Copy link
Member

Choose a reason for hiding this comment

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

I still don't know what that has to do with making rel=external not have the normal page reload nav behavior. The idea is that target or download or rel=external will make it a non SPA link even if it matches some route.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Conduitry True, I fixated on my case and shouldn't remove functionality for rel=external.

I've made PR only for the prefetch #1566 and will close this one.

### sapper:external

By default, the Sapper runtime intercepts clicks on `<a>` elements and bypasses the normal browser navigation for relative (same-origin) URLs that match one of your page routes. We sometimes need to tell Sapper that certain links need to be be handled by normal browser navigation.

Adding a `rel=external` attribute to a link...
Adding a `sapper:external` attribute to a link...

```html
<a rel=external href='path'>Path</a>
<a href='path' sapper:external>Path</a>
```

...will trigger a browser navigation when the link is clicked.
Expand Down
10 changes: 5 additions & 5 deletions site/src/routes/faq/index.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
</script>

<script>
const description = "Frequently Asked Questions about Sapper"
const description = "Frequently Asked Questions about Sapper";

export let faqs;
</script>
Expand All @@ -25,14 +25,14 @@

<article class='faq'>
<h2>
<span id={faq.fragment} class="offset-anchor"></span>
<a class="anchor" rel='prefetch' href='faq#{faq.fragment}' title='{faq.question}'>&nbsp;</a>
{faq.metadata.question}
<span id={faq.fragment} class="offset-anchor"></span>
<a class="anchor" sapper:prefetch href='faq#{faq.fragment}' title='{faq.question}'>&nbsp;</a>
{faq.metadata.question}
</h2>
<p>{@html faq.answer}</p>
</article>
{/each}
<p>See also the <a href="https://svelte.dev/faq" rel="external">Svelte FAQ</a> for questions relating to Svelte directly.</p>
<p>See also the <a href="https://svelte.dev/faq" sapper:external>Svelte FAQ</a> for questions relating to Svelte directly.</p>
</div>

<style>
Expand Down
4 changes: 2 additions & 2 deletions test/apps/preloading/src/routes/index.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

<a href="slow-preload">slow preload</a>
<a href="foo">foo</a>
<a href="prefetch/qwe" rel=prefetch>prefetch qwe</a>
<a href="prefetch/xyz" rel=prefetch>prefetch xyz</a>
<a href="prefetch/qwe" sapper:prefetch>prefetch qwe</a>
<a href="prefetch/xyz" sapper:prefetch>prefetch xyz</a>