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(gatsby-plugin-offline): use networkFirst caching for page-data.json files #14720

Merged
merged 2 commits into from
Jun 12, 2019
Merged

fix(gatsby-plugin-offline): use networkFirst caching for page-data.json files #14720

merged 2 commits into from
Jun 12, 2019

Conversation

Moocar
Copy link
Contributor

@Moocar Moocar commented Jun 11, 2019

Description

page-data.json files are not content hashed. Their value can change between builds and the filename will remain the same. Therefore, in the offline plugin, we should use networkFirst strategy.

Related Issues

@Moocar Moocar requested a review from a team as a code owner June 11, 2019 23:43
@DSchau DSchau changed the title In plugin-offline, use networkFirst caching for page-data.json files fix(gatsby-plugin-offline): use networkFirst caching for page-data.json files Jun 12, 2019
wardpeet
wardpeet reviewed Jun 12, 2019
View reviewed changes
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

url pattern seems to be working. Should we add a $ to the end of the regex?

I'll give it a spin just to make sure but looks great!

wardpeet
wardpeet approved these changes Jun 12, 2019
View reviewed changes
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Works like a charm. What do you think of adding the $ at the end of the regex?

@Moocar
Copy link
Contributor Author

Moocar commented Jun 12, 2019

Thanks for testing! I don't mind about the $ suffix. The regex is already quite specific. Would it help with performance at all?

@KyleAMathews
Copy link
Contributor

Yeah, seems fine without it. Merging!

@KyleAMathews KyleAMathews merged commit 5352411 into gatsbyjs:master Jun 12, 2019
mxxk pushed a commit to mxxk/gatsby that referenced this pull request Jun 21, 2019
@Moocar @mxxk
fix(gatsby-plugin-offline): use networkFirst caching for page-data.js…
92d41a0 
…on files (gatsbyjs#14720)

* In plugin-offline, use networkFirst caching for page-data.json files

* fix page-data offline regex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KyleAMathews KyleAMathews KyleAMathews left review comments

@wardpeet wardpeet wardpeet 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.

None yet
3 participants
@Moocar @KyleAMathews @wardpeet