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.

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

Version 2.24.70 Breaks How location.search Gets Updated #27351

Closed
adonig opened this issue Oct 9, 2020 · 8 comments
Closed

Version 2.24.70 Breaks How location.search Gets Updated #27351

adonig opened this issue Oct 9, 2020 · 8 comments
Assignees
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@adonig
Copy link

adonig commented Oct 9, 2020

Description

Starting with version 2.24.70 my website's search field stopped working.

Steps to reproduce

  1. Checkout the minimal reproduction
  2. Run yarn install && yarn run develop
  3. Open http://localhost:8000/ in the browser and try the search field. Note that it does work.
  4. Change "<2.24.70" in package.json to "^2.24.70"
  5. Run yarn install && yarn run develop
  6. Open http://localhost:8000/ in the browser and try the search field. Note that it does not work.

It looks like since 2.24.70 the changes to location.search don't cause the useEffect hook to execute.

  useEffect(() => {
    const newSearchQuery = new URLSearchParams(location.search).get(`q`) || ``
    setSearchQuery(newSearchQuery)
    if (newSearchQuery) {
      updateResults(newSearchQuery)
    }
  }, [location.search])

Expected result

The useEffect hook should run. The website should display the search results.

Actual result

It doesn't.

Environment

  System:
    OS: macOS 10.15.6
    CPU: (8) x64 Intel(R) Core(TM) i7-8559U CPU @ 2.70GHz
    Shell: 5.0.18 - /usr/local/bin/bash
  Binaries:
    Node: 14.8.0 - ~/.nvm/versions/node/v14.8.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.7 - ~/.nvm/versions/node/v14.8.0/bin/npm
  Languages:
    Python: 2.7.17 - /Users/asd/.pyenv/shims/python
  Browsers:
    Chrome: 86.0.4240.75
    Firefox: 81.0.1
    Safari: 14.0
  npmPackages:
    gatsby: ^2.24.70 => 2.24.72 
@adonig adonig added the type: bug An issue or pull request relating to a bug in Gatsby label Oct 9, 2020
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 9, 2020
@vladar vladar added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 9, 2020
@vladar
Copy link
Contributor

vladar commented Oct 9, 2020

I reproduced this locally. Probably caused by this: #27261 (comment)

@AndrewNatoli
Copy link

AndrewNatoli commented Oct 13, 2020

'ello! I have another example of this if it would help to have another reference. :)

A project you can clone + run locally (by default runs latest Gatsby and therefore the [[wiki-links]] do not work):

It's based off this project and you can see an example built with <2.24.70 here: https://github.com/mathieudutour/gatsby-digital-garden

If you put a breakpoint where stackedPagesSlugs is declared in react-stacked-pages-hook/hooks.tsx you'll find that it fires after clicking a [[wiki-link]] (due to the updated location) on the pre-2.24.70 example, but not the example I provided.

@WillMayger
Copy link
Contributor

WillMayger commented Oct 16, 2020

👋 Hi @vladar I am happy to pick this one up as it was my PR that seemed to cause it.

Just to double check my understanding though - the location search is currently not effecting the navigation when it should be because in the navigation we are using pathname as the comparison when we should be using the search query as well?

@vladar
Copy link
Contributor

vladar commented Oct 16, 2020

Hey @WillMayger ! That would be awesome! 💜

So yeah, I think the issue is that we should update on any location string change (so pathname+search+hash). The reproduction in this issue should help to understand if it was fixed for the search part. But keep in mind that we should also cover the hash case, not just pathname and search.

@WillMayger
Copy link
Contributor

👍 Brilliant thanks for the info, I will get started on a PR this weekend!

@WillMayger
Copy link
Contributor

👋 Hi Everyone, sorry I didn't manage to get round to it last weekend.

Please can I get a review on my PR which contains tests & a fix for this issue: #27594

Thanks

@vladar
Copy link
Contributor

vladar commented Oct 22, 2020

Thanks again! Published in gatsby@2.24.85

@adonig
Copy link
Author

adonig commented Oct 22, 2020

@vladar @WillMayger It works like a charm. Thank you very much.

@adonig adonig closed this as completed Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

No branches or pull requests

4 participants