Skip to content

[flutter_tools] optimize fetch requests and remove main.dart.js bypass #66069

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

Merged
merged 3 commits into from
Sep 18, 2020

Conversation

jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Sep 17, 2020

Description

The main.dart.js bypass is not needed now that we have skipWaiting. Additionally optimize the fetch handler so that resources not in the cache skip the service worker altogether.

Fixes #66068

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 17, 2020
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀 (I left a small nit)

Comment on lines +556 to 560
// If the URL is not the RESOURCE list then return to signal that the
// browser should take over.
if (!RESOURCES[key]) {
return event.respondWith(fetch(event.request));
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I was understanding "remove main.dart.js bypass" wrong. This lets the service worker cache the main.dart.js again, by not overriding its name with a hash.

(I was about to ask: if we want the serviceworker to bypass main.dart.js, shouldn't we also remove it from the RESOURCES map? :P)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly - now that I realize I was COMPLETE WRONG about the default behavior of sw, everything makes more sense!

But we also bypass the worker for http requests that won't go to the cache, since I guess there is overhead for forwarding the requests

@jonahwilliams jonahwilliams merged commit 84aae22 into flutter:master Sep 18, 2020
@jonahwilliams jonahwilliams deleted the sw_updates branch September 18, 2020 00:05
goderbauer pushed a commit to goderbauer/flutter that referenced this pull request Sep 18, 2020
flutter#66069)

The main.dart.js bypass is not needed now that we have skipWaiting. Additionally optimize the fetch handler so that resources not in the cache skip the service worker altogether.

Fixes flutter#66068
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Early return in fetch handler for requests that will not be handled with the service worker
3 participants