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

fix(gatsby-link): don't prefetch same page #28307

Merged
merged 4 commits into from
Nov 26, 2020
Merged

fix(gatsby-link): don't prefetch same page #28307

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
367c989
fix(gatsby-link): don't prefetch same page
wardpeet Nov 26, 2020
691665d
test prefect intersection-observer
wardpeet Nov 26, 2020
d0c5490
Merge remote-tracking branch 'upstream/master' into fix/wordpress-pre…
Nov 26, 2020
afafbf1
Merge remote-tracking branch 'upstream/master' into fix/wordpress-pre…
Nov 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/gatsby-link/src/__tests__/__snapshots__/index.js.snap
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ exports[`<Link /> matches basic snapshot 1`] = `
<div>
<a
class="link"
href="/"
href="/active"
TylerBarnes marked this conversation as resolved.
Show resolved Hide resolved
style="color: black;"
>
link
Expand Down
90 changes: 84 additions & 6 deletions packages/gatsby-link/src/__tests__/index.js
View file
Open in desktop
Expand Up @@ -44,7 +44,30 @@ const getWithAssetPrefix = (prefix = ``) => {
return withAssetPrefix
}

const setup = ({ sourcePath = `/active`, linkProps, pathPrefix = `` } = {}) => {
const setup = ({ sourcePath = `/`, linkProps, pathPrefix = `` } = {}) => {
let intersectionInstances = new WeakMap()
// mock intersectionObserver
global.IntersectionObserver = jest.fn(cb => {
let instance = {
observe: ref => {
intersectionInstances.set(ref, instance)
},
unobserve: ref => {
intersectionInstances.delete(ref)
},
disconnect: () => {},
trigger: ref => {
cb([
{
target: ref,
isIntersecting: true,
},
])
},
}

return instance
})
global.__BASE_PATH__ = pathPrefix
const source = createMemorySource(sourcePath)
const history = createHistory(source)
Expand All @@ -66,22 +89,31 @@ const setup = ({ sourcePath = `/active`, linkProps, pathPrefix = `` } = {}) => {

return Object.assign({}, utils, {
link: utils.getByText(`link`),
triggerInViewport: ref => {
intersectionInstances.get(ref).trigger(ref)
},
})
}

describe(`<Link />`, () => {
it(`matches basic snapshot`, () => {
const { container } = setup()
const { container } = setup({
linkProps: { to: `/active` },
})
expect(container).toMatchSnapshot()
})

it(`matches active snapshot`, () => {
const { container } = setup({ linkProps: { to: `/active` } })
const { container } = setup({
sourcePath: `/active`,
linkProps: { to: `/active` },
})
expect(container).toMatchSnapshot()
})

it(`matches partially active snapshot`, () => {
const { container } = setup({
sourcePath: `/active`,
linkProps: { to: `/active/nested`, partiallyActive: true },
})
expect(container).toMatchSnapshot()
Expand Down Expand Up @@ -150,19 +182,28 @@ describe(`<Link />`, () => {

it(`handles relative link with "./"`, () => {
const location = `./courses?sort=name`
const { link } = setup({ linkProps: { to: location } })
const { link } = setup({
sourcePath: `/active`,
linkProps: { to: location },
})
expect(link.getAttribute(`href`)).toEqual(`/active/courses?sort=name`)
})

it(`handles relative link with "../"`, () => {
const location = `../courses?sort=name`
const { link } = setup({ linkProps: { to: location } })
const { link } = setup({
sourcePath: `/active`,
linkProps: { to: location },
})
expect(link.getAttribute(`href`)).toEqual(`/courses?sort=name`)
})

it(`handles bare relative link`, () => {
const location = `courses?sort=name`
const { link } = setup({ linkProps: { to: location } })
const { link } = setup({
sourcePath: `/active`,
linkProps: { to: location },
})
expect(link.getAttribute(`href`)).toEqual(`/active/courses?sort=name`)
})

Expand Down Expand Up @@ -383,3 +424,40 @@ describe(`state`, () => {
)
})
})

describe(`prefetch`, () => {
beforeEach(() => {
global.___loader = {
enqueue: jest.fn(),
}
})

it(`it prefetches when in viewport`, () => {
const to = `/active`

const { link, triggerInViewport } = setup({
linkProps: { to },
})

triggerInViewport(link)

expect(global.___loader.enqueue).toHaveBeenCalledWith(
`${global.__BASE_PATH__}${to}`
)
})

it(`it does not prefetch if link is current page`, () => {
const to = `/active`

const { link, triggerInViewport } = setup({
sourcePath: `/active`,
linkProps: { to },
})

triggerInViewport(link)

expect(global.___loader.enqueue).not.toHaveBeenCalledWith(
`${global.__BASE_PATH__}${to}`
)
})
})
147 changes: 81 additions & 66 deletions packages/gatsby-link/src/index.js
View file
Open in desktop
Expand Up @@ -93,6 +93,14 @@ const createIntersectionObserver = (el, cb) => {
return { instance: io, el }
}

function GatsbyLinkLocationWrapper(props) {
return (
<Location>
{({ location }) => <GatsbyLink {...props} _location={location} />}
</Location>
)
}

class GatsbyLink extends React.Component {
constructor(props) {
super(props)
Expand All @@ -108,23 +116,35 @@ class GatsbyLink extends React.Component {
this.handleRef = this.handleRef.bind(this)
}

_prefetch() {
let currentPath = window.location.pathname

// reach router should have the correct state
if (this.props._location && this.props._location.pathname) {
TylerBarnes marked this conversation as resolved.
Show resolved Hide resolved
currentPath = this.props._location.pathname
}

const rewrittenPath = rewriteLinkPath(this.props.to, currentPath)
const newPathName = parsePath(rewrittenPath).pathname

// Prefech is used to speed up next navigations. When you use it on the current navigation,
// there could be a race-condition where Chrome uses the stale data instead of waiting for the network to complete
if (currentPath !== newPathName) {
___loader.enqueue(newPathName)
}
}

componentDidUpdate(prevProps, prevState) {
// Preserve non IO functionality if no support
if (this.props.to !== prevProps.to && !this.state.IOSupported) {
___loader.enqueue(
parsePath(rewriteLinkPath(this.props.to, window.location.pathname))
.pathname
)
this._prefetch()
}
}

componentDidMount() {
// Preserve non IO functionality if no support
if (!this.state.IOSupported) {
___loader.enqueue(
parsePath(rewriteLinkPath(this.props.to, window.location.pathname))
.pathname
)
this._prefetch()
}
}

Expand All @@ -148,10 +168,7 @@ class GatsbyLink extends React.Component {
if (this.state.IOSupported && ref) {
// If IO supported and element reference found, setup Observer functionality
this.io = createIntersectionObserver(ref, () => {
___loader.enqueue(
parsePath(rewriteLinkPath(this.props.to, window.location.pathname))
.pathname
)
this._prefetch()
})
}
}
Expand Down Expand Up @@ -181,70 +198,68 @@ class GatsbyLink extends React.Component {
partiallyActive,
state,
replace,
_location,
/* eslint-enable no-unused-vars */
...rest
} = this.props

if (process.env.NODE_ENV !== `production` && !isLocalLink(to)) {
console.warn(
`External link ${to} was detected in a Link component. Use the Link component only for internal links. See: https://gatsby.dev/internal-links`
)
}

const prefixedTo = rewriteLinkPath(to, _location.pathname)
if (!isLocalLink(prefixedTo)) {
return <a href={prefixedTo} {...rest} />
}

return (
<Location>
{({ location }) => {
const prefixedTo = rewriteLinkPath(to, location.pathname)
return isLocalLink(prefixedTo) ? (
<Link
to={prefixedTo}
state={state}
getProps={getProps}
innerRef={this.handleRef}
onMouseEnter={e => {
if (onMouseEnter) {
onMouseEnter(e)
}
___loader.hovering(parsePath(prefixedTo).pathname)
}}
onClick={e => {
if (onClick) {
onClick(e)
}

if (
e.button === 0 && // ignore right clicks
!this.props.target && // let browser handle "target=_blank"
!e.defaultPrevented && // onClick prevented default
!e.metaKey && // ignore clicks with modifier keys...
!e.altKey &&
!e.ctrlKey &&
!e.shiftKey
) {
e.preventDefault()

let shouldReplace = replace
const isCurrent =
encodeURI(prefixedTo) === window.location.pathname
if (typeof replace !== `boolean` && isCurrent) {
shouldReplace = true
}
// Make sure the necessary scripts and data are
// loaded before continuing.
window.___navigate(prefixedTo, {
state,
replace: shouldReplace,
})
}

return true
}}
{...rest}
/>
) : (
<a href={prefixedTo} {...rest} />
)
<Link
TylerBarnes marked this conversation as resolved.
Show resolved Hide resolved
to={prefixedTo}
state={state}
getProps={getProps}
innerRef={this.handleRef}
onMouseEnter={e => {
if (onMouseEnter) {
onMouseEnter(e)
}
___loader.hovering(parsePath(prefixedTo).pathname)
}}
onClick={e => {
if (onClick) {
onClick(e)
}

if (
e.button === 0 && // ignore right clicks
!this.props.target && // let browser handle "target=_blank"
!e.defaultPrevented && // onClick prevented default
!e.metaKey && // ignore clicks with modifier keys...
!e.altKey &&
!e.ctrlKey &&
!e.shiftKey
) {
e.preventDefault()

let shouldReplace = replace
const isCurrent = encodeURI(prefixedTo) === _location.pathname

if (typeof replace !== `boolean` && isCurrent) {
shouldReplace = true
}
// Make sure the necessary scripts and data are
// loaded before continuing.
window.___navigate(prefixedTo, {
state,
replace: shouldReplace,
})
}

return true
}}
</Location>
{...rest}
/>
)
}
}
Expand All @@ -263,7 +278,7 @@ const showDeprecationWarning = (functionName, altFunctionName, version) =>
)

export default React.forwardRef((props, ref) => (
<GatsbyLink innerRef={ref} {...props} />
<GatsbyLinkLocationWrapper innerRef={ref} {...props} />
))

export const navigate = (to, options) => {
Expand Down