-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
core(start_url): fix start_url audit while offline #4710
Conversation
93373be
to
5c9696c
Compare
431c97b
to
9ef1084
Compare
557b4f8
to
2a1a385
Compare
b9f6a72
to
cf70143
Compare
Some test urls that have failed before |
@wardpeet: not sure if @paulirish mentioned this to you somewhere else, but we'll have to stick to promises in this file for now because we want it in the 2.x branch, which doesn't drop Node 6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with this PR now. But final say to either patrick or brendan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this LGTM, fixes super impactful bug! but let's get that async/await cleanup in soon after ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM3
* Load manifest in online mode but execute fetch of start_url in offline
Tests are broken but I want some feedback on this one.
is this a good approach? It seems to work on some urls I've tested but should test more of them
It fixes #2576