Skip to content

Commit

Permalink
fix: do not prefetch for same url (QwikDev#1199)
Browse files Browse the repository at this point in the history
  • Loading branch information
@adamdbradley
adamdbradley authored Sep 3, 2022
1 parent 7988c6f commit 63c9bd5
Show file tree
Hide file tree
Showing 9 changed files with 313 additions and 170 deletions.
8 changes: 7 additions & 1 deletion packages/qwik-city/runtime/src/app/components/header/header.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,54 @@ export default component$(() => {
<header>
<div class="header-inner">
<section class="logo">
<Link href="/" data-test-link="header-home">
<Link href="/" prefetch={true} data-test-link="header-home">
Qwik City 🏙
</Link>
</section>
<nav data-test-header-links>
<Link
href="/blog"
prefetch={true}
class={mutable({ active: pathname.startsWith('/blog') })}
data-test-link="blog-home"
>
Blog
</Link>
<Link
href="/docs"
prefetch={true}
class={mutable({ active: pathname.startsWith('/docs') })}
data-test-link="docs-home"
>
Docs
</Link>
<Link
href="/api"
prefetch={true}
class={mutable({ active: pathname.startsWith('/api') })}
data-test-link="api-home"
>
API
</Link>
<Link
href="/products/hat"
prefetch={true}
class={mutable({ active: pathname.startsWith('/products') })}
data-test-link="products-hat"
>
Products
</Link>
<Link
href="/about-us"
prefetch={true}
class={mutable({ active: pathname.startsWith('/about-us') })}
data-test-link="about-us"
>
About Us
</Link>
<Link
href="/sign-in"
prefetch={true}
class={mutable({ active: pathname.startsWith('/sign-in') })}
data-test-link="sign-in"
>
Expand Down
60 changes: 4 additions & 56 deletions .../runtime/src/library/client-navigation.ts → ...ty/runtime/src/library/client-navigate.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type { RouteNavigate } from './types';
import type { RouteNavigate, SimpleURL } from './types';
import { isSameOriginDifferentPathname, isSamePath, toPath, toUrl } from './utils';

export const clientNavigate = (win: ClientHistoryWindow, routeNavigate: RouteNavigate) => {
const currentUrl = win.location;
const newUrl = toUrl(routeNavigate.path, currentUrl)!;

if (isSameOriginDifferentPath(currentUrl, newUrl)) {
if (isSameOriginDifferentPathname(currentUrl, newUrl)) {
// current browser url and route path are different
// see if we should scroll to the hash after the url update
handleScroll(win, currentUrl, newUrl);
Expand All @@ -22,7 +23,7 @@ export const clientNavigate = (win: ClientHistoryWindow, routeNavigate: RouteNav
const currentUrl = win.location;
const previousUrl = toUrl(routeNavigate.path, currentUrl)!;

if (isSameOriginDifferentPath(currentUrl, previousUrl)) {
if (isSameOriginDifferentPathname(currentUrl, previousUrl)) {
handleScroll(win, previousUrl, currentUrl);
// current browser url and route path are different
// update the route path
Expand All @@ -32,51 +33,6 @@ export const clientNavigate = (win: ClientHistoryWindow, routeNavigate: RouteNav
}
};

/**
* Gets an absolute url path string (url.pathname + url.search + url.hash)
*/
export const toPath = (url: SimpleURL) => url.pathname + url.search + url.hash;

/**
* Create a URL from a string and baseUrl
*/
export const toUrl = (url: string, baseUrl: { href: string }) => new URL(url, baseUrl.href);

/**
* Checks only if the origins are the same.
*/
const isSameOrigin = (a: SimpleURL, b: SimpleURL) => a.origin === b.origin;

/**
* Checks only if the pathname + search are the same for the URLs.
*/
const isSamePath = (a: SimpleURL, b: SimpleURL) => toPath(a) === toPath(b);

/**
* Same origin, but different pathname + search + hash.
*/
export const isSameOriginDifferentPath = (a: SimpleURL, b: SimpleURL) =>
isSameOrigin(a, b) && !isSamePath(a, b);

export const getClientNavPath = (props: Record<string, any>, baseUrl: { href: string }) => {
const href = props.href;
if (typeof href === 'string' && href.trim() !== '' && typeof props.target !== 'string') {
try {
const linkUrl = toUrl(href, baseUrl);
const currentUrl = toUrl('', baseUrl)!;
if (isSameOrigin(linkUrl, currentUrl)) {
return toPath(linkUrl);
}
} catch (e) {
console.error(e);
}
}
return null;
};

export const getClientEndpointPath = (pathname: string) =>
pathname + (pathname.endsWith('/') ? '' : '/') + 'q-data.json';

const handleScroll = async (win: Window, previousUrl: SimpleURL, newUrl: SimpleURL) => {
const doc = win.document;
const newHash = newUrl.hash;
Expand Down Expand Up @@ -137,11 +93,3 @@ export const CLIENT_HISTORY_INITIALIZED = /* @__PURE__ */ Symbol();
export interface ClientHistoryWindow extends Window {
[CLIENT_HISTORY_INITIALIZED]?: 1;
}

export interface SimpleURL {
origin: string;
href: string;
pathname: string;
search: string;
hash: string;
}
107 changes: 4 additions & 103 deletions ...ime/src/library/client-navigation.unit.ts → ...ntime/src/library/client-navigate.unit.ts
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
import { test, suite } from 'uvu';
import { suite } from 'uvu';
import { equal } from 'uvu/assert';
import {
ClientHistoryWindow,
clientNavigate,
CLIENT_HISTORY_INITIALIZED,
getClientEndpointPath,
getClientNavPath,
isSameOriginDifferentPath,
SimpleURL,
toPath,
} from './client-navigation';
import type { RouteNavigate } from './types';
import { ClientHistoryWindow, clientNavigate, CLIENT_HISTORY_INITIALIZED } from './client-navigate';
import type { RouteNavigate, SimpleURL } from './types';
import { toPath } from './utils';

const navTest = suite('clientNavigate');

Expand Down Expand Up @@ -152,94 +144,3 @@ function createRouteNavigate(win: { location: SimpleURL }) {
}

navTest.run();

test('isSameOriginDifferentPath', () => {
const compare = [
{
a: 'http://qwik.dev/',
b: 'http://qwik.dev/',
expect: false,
},
{
a: 'http://qwik.dev/',
b: 'http://b.io/',
expect: false,
},
{
a: 'http://qwik.dev/',
b: 'http://b.io/path-b',
expect: false,
},
{
a: 'http://qwik.dev/path-a',
b: 'http://qwik.dev/path-b',
expect: true,
},
{
a: 'http://qwik.dev/qs=a',
b: 'http://qwik.dev/qs=b',
expect: true,
},
{
a: 'http://qwik.dev/qs=a',
b: 'http://qwik.dev/qs=a',
expect: false,
},
{
a: 'http://qwik.dev/qs=a#hash1',
b: 'http://qwik.dev/qs=b#hash1',
expect: true,
},
{
a: 'http://qwik.dev/qs=a#hash1',
b: 'http://qwik.dev/qs=a#hash1',
expect: false,
},
{
a: 'http://qwik.dev/qs=a#hash1',
b: 'http://qwik.dev/qs=a#hash2',
expect: true,
},
];

compare.forEach((c) => {
const a = new URL(c.a);
const b = new URL(c.b);
equal(isSameOriginDifferentPath(a, b), c.expect, `${a} ${b}`);
});
});

const baseUrl = new URL('https://qwik.dev/');
[
{ props: { href: '#hash' }, expect: '/#hash' },
{ props: { href: '?qs=true' }, expect: '/?qs=true' },
{ props: { href: '/abs-path' }, expect: '/abs-path' },
{ props: { href: './rel-path' }, expect: '/rel-path' },
{ props: { href: 'rel-path' }, expect: '/rel-path' },
{ props: { href: '/path/../rel-path' }, expect: '/rel-path' },
{ props: { href: '/abs-path', target: '_blank' }, expect: null },
{ props: { href: 'http://qwik.dev/' }, expect: null },
{ props: { href: 'http://builder.io/' }, expect: null },
{ props: { href: ' ' }, expect: null },
{ props: { href: ' ' }, expect: null },
{ props: { href: '' }, expect: null },
{ props: { href: null }, expect: null },
{ props: {}, expect: null },
].forEach((c) => {
test(`getClientNavPath ${c.props.href}`, () => {
equal(getClientNavPath(c.props, baseUrl), c.expect, `${c.props.href} ${c.expect}`);
});
});

[
{ pathname: '/', expect: '/q-data.json' },
{ pathname: '/about', expect: '/about/q-data.json' },
{ pathname: '/about/', expect: '/about/q-data.json' },
].forEach((t) => {
test(`getClientEndpointUrl("${t.pathname}")`, () => {
const endpointPath = getClientEndpointPath(t.pathname);
equal(endpointPath, t.expect);
});
});

test.run();
16 changes: 8 additions & 8 deletions packages/qwik-city/runtime/src/library/link-component.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { component$, Slot, QwikIntrinsicElements } from '@builder.io/qwik';
import { getClientNavPath, toUrl } from './client-navigation';
import { getClientNavPath, getPrefetchUrl } from './utils';
import { loadClientData } from './use-endpoint';
import { useLocation, useNavigate } from './use-functions';

Expand All @@ -11,17 +11,17 @@ export const Link = component$<LinkProps>((props) => {
const loc = useLocation();
const originalHref = props.href;
const linkProps = { ...props };
const clientPathname = getClientNavPath(linkProps, loc);
const prefetchUrl = props.prefetch && clientPathname ? toUrl(clientPathname, loc).href : null;
const clientNavPath = getClientNavPath(linkProps, loc);
const prefetchUrl = getPrefetchUrl(props, clientNavPath, loc);

linkProps['preventdefault:click'] = !!clientPathname;
linkProps.href = clientPathname || originalHref;
linkProps['preventdefault:click'] = !!clientNavPath;
linkProps.href = clientNavPath || originalHref;

return (
<a
{...linkProps}
onClick$={() => {
if (clientPathname) {
if (clientNavPath) {
nav.path = linkProps.href!;
}
}}
Expand All @@ -33,8 +33,6 @@ export const Link = component$<LinkProps>((props) => {
);
});

let windowInnerWidth = 0;

export const prefetchLinkResources = (prefetchUrl: string | null, isOnVisible: boolean) => {
if (!windowInnerWidth) {
windowInnerWidth = window.innerWidth;
Expand All @@ -47,6 +45,8 @@ export const prefetchLinkResources = (prefetchUrl: string | null, isOnVisible: b
}
};

let windowInnerWidth = 0;

type AnchorAttributes = QwikIntrinsicElements['a'];

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/qwik-city/runtime/src/library/qwik-city-component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ import {
import { createDocumentHead, resolveHead } from './head';
import { isBrowser, isServer } from '@builder.io/qwik/build';
import { useQwikCityEnv } from './use-functions';
import { clientNavigate, toPath } from './client-navigation';
import { clientNavigate } from './client-navigate';
import { loadClientData } from './use-endpoint';
import { toPath } from './utils';

/**
* @alpha
Expand Down
8 changes: 8 additions & 0 deletions packages/qwik-city/runtime/src/library/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,11 @@ export interface QwikCityEnvData {
}

export type GetEndpointData<T> = T extends RequestHandler<infer U> ? U : T;

export interface SimpleURL {
origin: string;
href: string;
pathname: string;
search: string;
hash: string;
}
2 changes: 1 addition & 1 deletion packages/qwik-city/runtime/src/library/use-endpoint.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useResource$ } from '@builder.io/qwik';
import { useLocation, useQwikCityEnv } from './use-functions';
import { isServer } from '@builder.io/qwik/build';
import type { ClientPageData, GetEndpointData } from './types';
import { getClientEndpointPath } from './client-navigation';
import { getClientEndpointPath } from './utils';
import type { QPrefetchData } from './service-worker/types';
import { cacheModules } from '@qwik-city-plan';

Expand Down
Loading

0 comments on commit 63c9bd5

Please sign in to comment.