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): Improve reliability of JS detection #18760

Merged
merged 6 commits into from
Nov 4, 2019
Merged

fix(gatsby-plugin-offline): Improve reliability of JS detection #18760

merged 6 commits into from
Nov 4, 2019

Conversation

vtenfys
Copy link
Contributor

@vtenfys vtenfys commented Oct 17, 2019

Description

Uses a similar but slightly different approach to JS disabled detection, which doesn't rely on preload working. Fixes issue with the API not being called on Firefox.

@vtenfys vtenfys requested a review from a team as a code owner October 17, 2019 10:26
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.

Looks great! thanks for making it more resilient. Can you add a few comments to the code so we can get this merged! 💪

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.

This is great, thanks!

Additional improvement could to try to make use of request.referrer if available over lastNavigationRequest (which can have unlikely timing issues given that it is global variable that can be changed by unrelated request in different tab or so):

Screenshot 2019-11-04 at 14 38 46

But this might introduce additional edge cases, so we should merge this in and research potential usage of referrer in follow up

@pieh pieh added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Nov 4, 2019
@gatsbybot gatsbybot merged commit ae6eab3 into gatsbyjs:master Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants