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(www): avoid querying for no-cache=1 #10389

Merged
merged 1 commit into from
Dec 12, 2018
Merged

fix(www): avoid querying for no-cache=1 #10389

merged 1 commit into from
Dec 12, 2018

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Dec 10, 2018

overall this could also have been done by parsing & stringifying with qs, but this should work too.

I haven't been able to try this out yet (I didn't have the patience to wait for all the images to transform ahah), but I don't think this can break anything.

Changes done:

  1. small refactors to make code a little shorter
  2. replace no-cache=1 from the query before searching.

I noticed when seeing a lot of o-cache=1 hits on Algolia analytics

@Haroenv
fix(www): avoid querying for no-cache=1
b843b0b 
overall this could also have been done by parsing & stringifying with qs, but this should work too.

I haven't been able to try this out yet (I didn't have the patience to wait for all the images to transform ahah), but I don't think this can break anything
@Haroenv Haroenv requested a review from a team as a code owner December 10, 2018 16:44
@pieh
Copy link
Contributor

pieh commented Dec 10, 2018

I haven't been able to try this out yet (I didn't have the patience to wait for all the images to transform ahah), but I don't think this can break anything.

You can run GATSBY_SCREENSHOT_PLACEHOLDER=true gatsby develop which will skip pulling screenshots for site/starter showcases (which is like 75% of all the images on gatsbyjs.org)

@pieh
Copy link
Contributor

pieh commented Dec 10, 2018 •
edited
Loading

We did remove no-cache stuff recently (moved to different implementation) - do you still get this when browsing gatsbyjs.org?

@Haroenv
Copy link
Contributor Author

Haroenv commented Dec 11, 2018

I don't know how the URL gets there, but I see recent calls to that query in Algolia: o-cache=1 is the #1 query without results on the index over the last week still. Also last day still.

@btk
Copy link
Contributor

btk commented Dec 12, 2018

@pieh I come across this problem while coming from google.

Seems like google has indexed some links with this query. After the new implementation its not relevant but, this pr might be helpful.

screenshot at dec 12 03-50-49

@Haroenv
Copy link
Contributor Author

Haroenv commented Dec 12, 2018

I could make a small implementation that serialises to ?q=xxx. Which browsers are supposed to be supported in this part of the site, is new Map(new URLSearchParams(location.search)) acceptable (only latest browsers), or would a more compatible (i.e. using qs or manual parsing) way be preferred

@pieh
Copy link
Contributor

pieh commented Dec 12, 2018

react@16 requires Map and Set, so it would work without extra polyfills (because they are already polifilled), but I don't think we need to do anything more complex here. Did quick tests and works as expected, so I'll merge as-is.

pieh
pieh approved these changes Dec 12, 2018
View reviewed changes
Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Haroenv!

@pieh pieh merged commit 45d2832 into gatsbyjs:master Dec 12, 2018
@carlrosell carlrosell mentioned this pull request Dec 13, 2018
Merged
pieh pushed a commit that referenced this pull request Dec 13, 2018
@carlrosell @pieh
fix(www) Fix query for plugin links always ?=undefined (#10453)
ec3487c 
Was just browsing the website and found that #10389 PR changed the prop passed to the Result component to query but did not update the Result component itself.

https://github.com/gatsbyjs/gatsby/pull/10389/files#diff-86b9922590ad1703ea1c4ae10baab77cR252
m-allanson added a commit to Bouncey/gatsby that referenced this pull request Dec 14, 2018
@m-allanson
Merge branch 'master' into feat/no-watch-on-build
18f60c2 
* master: (33 commits)
  fix(blog): youfit case study typofix
  Doc improvements to Visual testing with Storybook guide (gatsbyjs#10436)
  fix(gatsby-plugin-offline): prevent incorrect revisioning of static file by workbox (gatsbyjs#10416)
  fix(starters): ttag repo link
  fix typo in pull request template (gatsbyjs#10454)
  fix(www) Fix query for plugin links always ?=undefined (gatsbyjs#10453)
  chore(release): Publish
  fix(gatsby): fix extracting StaticQuery nested in shorthand fragment (gatsbyjs#10443)
  fix(www): avoid querying for no-cache=1 (gatsbyjs#10389)
  fix(gatsby-image): update typescript definitions - properly mark fields as optional (gatsbyjs#10419)
  refactor(gatsby): improve EnsureResources (gatsbyjs#10224)
  Fixed minor Typos and grammatical errors (gatsbyjs#9353)
  docs: add ClinciJS website into showcase (gatsbyjs#10437)
  docs(babel-preset-gatsby): document --save-dev flag in README (gatsbyjs#10434)
  fix(docs): Environment Variables Examples (gatsbyjs#10406)
  chore(release): Publish
  [gatsby-image] re: fade out base64 on full image load (gatsbyjs#7539)
  docs(starter-library): add example to starter library (gatsbyjs#10425)
  docs(gatsby-plugin-offline): specify to not HTTP-cache sw.js (gatsbyjs#10430)
  fix(docs): prompt => confirm (gatsbyjs#10431)
  ...
m-allanson added a commit to lipis/gatsby that referenced this pull request Dec 17, 2018
@m-allanson
Merge branch 'master' into svgo
55f39a7 
* master: (1037 commits)
  Update starters.yml (gatsbyjs#10505)
  chore(release): Publish
  fix(graphql-skip-limit): fix hasNextPage (gatsbyjs#10504)
  chore: use cjs instead of esm for consistency (gatsbyjs#10494)
  feat(gatsby-remark-copy-linked-files): add support for video elements with `src` attribute (gatsbyjs#10395)
  typofix (gatsbyjs#10488)
  Add kobit.in to showcase (gatsbyjs#10496)
  fix(docs): window.reload => window.location.reload (gatsbyjs#10459)
  feat(www): add unbird feedback component to starter lib (gatsbyjs#10450)
  fix(blog): youfit case study typofix
  Doc improvements to Visual testing with Storybook guide (gatsbyjs#10436)
  fix(gatsby-plugin-offline): prevent incorrect revisioning of static file by workbox (gatsbyjs#10416)
  fix(starters): ttag repo link
  fix typo in pull request template (gatsbyjs#10454)
  fix(www) Fix query for plugin links always ?=undefined (gatsbyjs#10453)
  chore(release): Publish
  fix(gatsby): fix extracting StaticQuery nested in shorthand fragment (gatsbyjs#10443)
  fix(www): avoid querying for no-cache=1 (gatsbyjs#10389)
  fix(gatsby-image): update typescript definitions - properly mark fields as optional (gatsbyjs#10419)
  refactor(gatsby): improve EnsureResources (gatsbyjs#10224)
  ...
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
@Haroenv @gpetrioli
fix(www): avoid querying for no-cache=1 (gatsbyjs#10389)
bdd70e5
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
@carlrosell @gpetrioli
fix(www) Fix query for plugin links always ?=undefined (gatsbyjs#10453)
42d070c 
Was just browsing the website and found that gatsbyjs#10389 PR changed the prop passed to the Result component to query but did not update the Result component itself.

https://github.com/gatsbyjs/gatsby/pull/10389/files#diff-86b9922590ad1703ea1c4ae10baab77cR252
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pieh pieh pieh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Haroenv @pieh @btk