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

wardpeet · gatsbybot · web-flow · commit 36661308db47 · 2020-11-26T19:53:32.000+01:00
* fix(gatsby-link): don't prefetch same page

* test prefect intersection-observer

Co-authored-by: gatsbybot &lt;mathews.kyle+gatsbybot@gmail.com&gt;
diff --git a/packages/gatsby-link/src/__tests__/__snapshots__/index.js.snap b/packages/gatsby-link/src/__tests__/__snapshots__/index.js.snap
@@ -17,7 +17,7 @@ exports[`<Link /> matches basic snapshot 1`] = `
 <div>
   <a
     class="link"
-    href="/"
+    href="/active"
     style="color: black;"
   >
     link
diff --git a/packages/gatsby-link/src/__tests__/index.js b/packages/gatsby-link/src/__tests__/index.js
@@ -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)
@@ -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()
@@ -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`)
     })
 
@@ -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}`
+    )
+  })
+})
diff --git a/packages/gatsby-link/src/index.js b/packages/gatsby-link/src/index.js
@@ -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)
@@ -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) {
+      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()
     }
   }
 
@@ -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()
       })
     }
   }
@@ -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
+        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}
+      />
     )
   }
 }
@@ -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) => {

Original file line number	Diff line number	Diff line change
@@ -17,7 +17,7 @@ exports[`<Link /> matches basic snapshot 1`] = `
`17`	`17`	`<div>`
`18`	`18`	`<a`
`19`	`19`	`class="link"`
`20`		`- href="/"`
	`20`	`+ href="/active"`
`21`	`21`	`style="color: black;"`
`22`	`22`	`>`
`23`	`23`	`link`