Skip to content

Commit

Permalink
fix(gatsby): re-render route when location state changes (#28346) (#2…
Browse files Browse the repository at this point in the history
…8476)

* creating e2e tests to catch issue

* comparing location and state

* update test message

* using optional chaining

(cherry picked from commit 3ccaec8)

Co-authored-by: Will Mayger <[email protected]>
  • Loading branch information
GatsbyJS Bot and WillMayger authored Dec 4, 2020
1 parent 7a6b7ae commit bb044de
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,31 @@ describe(`navigation`, () => {
})

it(`should trigger an effect after a search param has changed`, () => {
cy.findByTestId(`effect-message`).should(`have.text`, `Waiting for effect`)
cy.findByTestId(`effect-message`).should(
`have.text`,
`Waiting for effect`
)
cy.findByTestId(`send-search-message`).click().waitForRouteChange()
cy.findByTestId(`effect-message`).should(`have.text`, `?message=searchParam`)
cy.findByTestId(`effect-message`).should(
`have.text`,
`?message=searchParam`
)
})

it(`should trigger an effect after the hash has changed`, () => {
cy.findByTestId(`effect-message`).should(`have.text`, `Waiting for effect`)
cy.findByTestId(`effect-message`).should(
`have.text`,
`Waiting for effect`
)
cy.findByTestId(`send-hash-message`).click().waitForRouteChange()
cy.findByTestId(`effect-message`).should(`have.text`, `#message-hash`)
})

it(`should trigger an effect after the state has changed`, () => {
cy.findByTestId(`effect-message`).should(`have.text`, ``)
cy.findByTestId(`send-state-message`).click().waitForRouteChange()
cy.findByTestId(`effect-message`).should(`have.text`, `this is a message using the state`)
})
})
}

Expand All @@ -173,14 +188,23 @@ describe(`navigation`, () => {
it(`should trigger an effect after a search param has changed`, () => {
cy.findByTestId(`effect-message`).should(`have.text`, ``)
cy.findByTestId(`send-search-message`).click().waitForRouteChange()
cy.findByTestId(`effect-message`).should(`have.text`, `?message=searchParam`)
cy.findByTestId(`effect-message`).should(
`have.text`,
`?message=searchParam`
)
})

it(`should trigger an effect after the hash has changed`, () => {
cy.findByTestId(`effect-message`).should(`have.text`, ``)
cy.findByTestId(`send-hash-message`).click().waitForRouteChange()
cy.findByTestId(`effect-message`).should(`have.text`, `#message-hash`)
})

it(`should trigger an effect after the state has changed`, () => {
cy.findByTestId(`effect-message`).should(`have.text`, ``)
cy.findByTestId(`send-state-message`).click().waitForRouteChange()
cy.findByTestId(`effect-message`).should(`have.text`, `this is a message using the state`)
})
})
}

Expand Down
19 changes: 18 additions & 1 deletion e2e-tests/development-runtime/src/pages/navigation-effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default function NavigationEffects({ location }) {

const searchParam = location.search
const searchHash = location.hash
const searchState = location?.state?.message

useEffect(() => {
setMessage(searchParam)
Expand All @@ -17,7 +18,12 @@ export default function NavigationEffects({ location }) {
setMessage(searchHash)
}, [searchHash])

const handleClick = next => navigate(`${next}`, { replace: true })
useEffect(() => {
setMessage(searchState)
}, [searchState])

const handleClick = (next, options = { replace: true }) =>
navigate(`${next}`, options)

return (
<Layout>
Expand All @@ -35,6 +41,17 @@ export default function NavigationEffects({ location }) {
>
Send Hash
</button>

<button
data-testid="send-state-message"
onClick={() =>
handleClick("/navigation-effects", {
state: { message: "this is a message using the state" },
})
}
>
Send state
</button>
</Layout>
)
}
17 changes: 14 additions & 3 deletions packages/gatsby/cache-dir/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,18 @@ class RouteAnnouncer extends React.Component {
}
}

const compareLocationProps = (prevLocation, nextLocation) => {
if (prevLocation.href !== nextLocation.href) {
return true
}

if (prevLocation?.state?.key !== nextLocation?.state?.key) {
return true
}

return false
}

// Fire on(Pre)RouteUpdate APIs
class RouteUpdates extends React.Component {
constructor(props) {
Expand All @@ -210,16 +222,15 @@ class RouteUpdates extends React.Component {
}

shouldComponentUpdate(prevProps) {
if (this.props.location.href !== prevProps.location.href) {
if (compareLocationProps(prevProps.location, this.props.location)) {
onPreRouteUpdate(this.props.location, prevProps.location)
return true
}

return false
}

componentDidUpdate(prevProps) {
if (this.props.location.href !== prevProps.location.href) {
if (compareLocationProps(prevProps.location, this.props.location)) {
onRouteUpdate(this.props.location, prevProps.location)
}
}
Expand Down

0 comments on commit bb044de

Please sign in to comment.