Skip to content

Commit

Permalink
fix: enable ref forwarding with forwardRef (#9892)
Browse files Browse the repository at this point in the history
Note: I think we should minimally make this more than a patch release per
[React docs](https://reactjs.org/docs/forwarding-refs.html#note-for-component-library-maintainers)

This will make the Link component work more like many would expect
(innerRef is confusing!), but it does come at the cost of requiring
React 16.3. I think it's a reasonable trade off, however.

cc @LekoArts I validated this with a sample repo, but want to pull this down and make sure it fixes your issue, as well?

<!--
  Q. Which branch should I use for my pull request?
  A. Use `master` branch (probably).

  Q. Which branch if my change is a bug fix for Gatsby v1?
  A. In this case, you should use the `v1` branch

  Q. Which branch if I'm still not sure?
  A. Use `master` branch. Ask in the PR if you're not sure and a Gatsby maintainer will be happy to help :)

  Note: We will only accept bug fixes for Gatsby v1. New features should be added to Gatsby v2.

  Learn more about contributing: https://www.gatsbyjs.org/docs/how-to-contribute/
-->
  • Loading branch information
DSchau committed Jan 10, 2019
1 parent d337e63 commit b6d9775
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 6 deletions.
4 changes: 2 additions & 2 deletions docs/docs/gatsby-link.md
Expand Up @@ -20,7 +20,7 @@ Preloading is triggered by a link entering the viewport; Gatsby uses
`Link`'s `innerRef` property to create a new IntersectionObserver (on
supported browsers) to monitor visible links. This way, Gatsby only prefetches
code/data chunks for pages the user is likely to navigate to. You can also get
access to the link element by passing in a `innerRef` prop.
access to the link element by passing in a `ref` prop, which will be forwarded to the `@reach/router` `Link` element directly.

## How to use

Expand All @@ -39,7 +39,7 @@ class Page extends React.Component {
activeStyle={{
color: "red",
}}
innerRef={el => {
ref={el => {
this.myLink = el
}}
state={{
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby-link/package.json
Expand Up @@ -26,7 +26,8 @@
"license": "MIT",
"main": "index.js",
"peerDependencies": {
"gatsby": ">2.0.0-alpha"
"gatsby": ">2.0.0",
"react": ">=16.3"
},
"repository": "https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-link",
"scripts": {
Expand Down
18 changes: 18 additions & 0 deletions packages/gatsby-link/src/__tests__/index.js
Expand Up @@ -147,3 +147,21 @@ describe(`navigate`, () => {
)
})
})

describe(`ref forwarding`, () => {
it(`forwards ref`, () => {
const ref = jest.fn()
setup({ linkProps: { ref } })

expect(ref).toHaveBeenCalledTimes(1)
expect(ref).toHaveBeenCalledWith(expect.any(HTMLElement))
})

it(`remains backwards compatible with innerRef`, () => {
const innerRef = jest.fn()
setup({ linkProps: { innerRef } })

expect(innerRef).toHaveBeenCalledTimes(1)
expect(innerRef).toHaveBeenCalledWith(expect.any(HTMLElement))
})
})
7 changes: 4 additions & 3 deletions packages/gatsby-link/src/index.js
Expand Up @@ -38,7 +38,7 @@ const handleIntersection = (el, cb) => {

class GatsbyLink extends React.Component {
constructor(props) {
super()
super(props)
// Default to no support for IntersectionObserver
let IOSupported = false
if (typeof window !== `undefined` && window.IntersectionObserver) {
Expand Down Expand Up @@ -97,7 +97,6 @@ class GatsbyLink extends React.Component {
/* eslint-disable no-unused-vars */
activeClassName: $activeClassName,
activeStyle: $activeStyle,
ref: $ref,
innerRef: $innerRef,
state,
replace,
Expand Down Expand Up @@ -154,7 +153,9 @@ GatsbyLink.propTypes = {
replace: PropTypes.bool,
}

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

export const navigate = (to, options) => {
window.___navigate(withPrefix(to), options)
Expand Down

0 comments on commit b6d9775

Please sign in to comment.