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

fix(gatsby-plugin-offline): skip prefetching all resources #16691

Merged
merged 4 commits into from Sep 24, 2019

Conversation

@rexxars
Copy link
Member

rexxars commented Aug 16, 2019

Description

If you have a plugin which uses the setHeadComponents API to add link-tags with a preconnect rel, the offline plugin will schedule it to be prefetched. This leads to the situation where instead of simply opening a connection to the specified host, it instead triggers a GET-request.

I'm not 100% clear on what should happen if you have prefetch or prerender links, but I feel like skipping them is probably a better option that to add another prefetch for them? Would love to hear your thoughts on this, though.

Related Issues

Fixes #15883

@rexxars rexxars requested a review from kkemple Aug 16, 2019
@rexxars rexxars requested a review from gatsbyjs/core as a code owner Aug 16, 2019
Copy link
Member

wardpeet left a comment

This is a great change. This will fix over fetching assets which is great! I added a few comments which I think makes this change a little bit better. Feel free to decide otherwise 😉 I'm happy to hear your thoughts.

packages/gatsby-plugin-offline/src/gatsby-browser.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-offline/src/gatsby-browser.js Outdated Show resolved Hide resolved
@LekoArts

This comment has been minimized.

Copy link
Contributor

LekoArts commented Sep 16, 2019

Hi @rexxars 👋 Did you find time to address Ward's comments? Would be awesome to have this merged in! Thanks a lot :)

@rexxars

This comment has been minimized.

Copy link
Member Author

rexxars commented Sep 16, 2019

Will have a look later today!

Copy link
Member

wardpeet left a comment

LGTM after changes! 👍 Thanks, this is a great change! We reduce bandwidth and lower the amount of logs saying that a preload has been triggered but left unused.

@rexxars

This comment has been minimized.

Copy link
Member Author

rexxars commented Sep 23, 2019

Thanks for fixing, I kind of forgot about this 😊

@wardpeet

This comment has been minimized.

Copy link
Member

wardpeet commented Sep 24, 2019

I'm going to merge this one, if we need to rollback, we'll rollback :)

@wardpeet wardpeet changed the title fix(gatsby-plugin-offline): skip prefetching preconnect resources fix(gatsby-plugin-offline): skip prefetching all resources Sep 24, 2019
@wardpeet wardpeet merged commit e688b0c into gatsbyjs:master Sep 24, 2019
20 checks passed
20 checks passed
Danger All good
Details
Peril All green. Congrats.
Details
ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_path-prefix Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_pipeline Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_long_term_caching Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: starters_validate Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node10 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node12 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node8 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_www Your tests passed on CircleCI!
Details
ci/circleci: windows_unit_tests Your tests passed on CircleCI!
Details
cypress: default-group 67 tests passed in 00:51
Details
unit_tests_windows Build #20190923.66 succeeded
Details
@rexxars rexxars deleted the rexxars:offline-dont-fetch-preconnect branch Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.