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

Adding the route logic #893

Merged
merged 3 commits into from
Oct 14, 2017
Merged

Adding the route logic #893

merged 3 commits into from
Oct 14, 2017

Conversation

gauntface
Copy link

R: @jeffposnick @addyosmani

This implements #886 minus tests. Wanted to get a gut check on the approach and will add tests in a follow up PR.

@gauntface gauntface requested a review from jeffposnick October 13, 2017 16:25
*
* @return {Array<string>} An array of URLs.
*/
async getCachedUrls() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking that it would be safer to always go through the steps of creating a fresh array of URLs and not saving and reusing the previous this._parsedCacheUrls value, since the underlying this._entriesToCacheMap value might be changed at any time.

Alternatively, you could set this._parsedCacheUrls to null whenever this._entriesToCacheMap is modified, but that's probably overkill.

(This might have been an issue with v2, too.)

Copy link
Author

@gauntface gauntface Oct 13, 2017

Choose a reason for hiding this comment

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

Just changed it to always return a fresh array.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@workbox-pr-bot
Copy link
Collaborator

PR-Bot Size Plugin

Changed File Sizes

File Before After Change GZipped
packages/workbox-core/build/browser/workbox-core.prod.js 3.99 KB 4.08 KB +2% 1.81 KB
packages/workbox-precaching/build/browser/workbox-precaching.prod.js 3.08 KB 3.66 KB +19% 1.59 KB ☠️

New Files

No new files have been added.

All File Sizes

View Table
File Before After Change GZipped
packages/workbox-core/build/browser/workbox-core.prod.js 3.99 KB 4.08 KB +2% 1.81 KB
packages/workbox-precaching/build/browser/workbox-precaching.prod.js 3.08 KB 3.66 KB +19% 1.59 KB ☠️
packages/workbox-routing/build/browser/workbox-routing.prod.js 1.48 KB 1.48 KB 0% 754 B
packages/workbox-runtime-caching/build/browser/workbox-runtime-caching.prod.js 1.81 KB 1.81 KB 0% 660 B
packages/workbox-sw/build/browser/workbox-sw.prod.js 1.28 KB 1.28 KB 0% 649 B

Workbox Aggregate Size Plugin

⚠️ WARNING ⚠️

We are using 92.13% of our max size budget.

Total Size: 9KB
Percentage of Size Used: 92%

Gzipped: 4.06KB

@addyosmani
Copy link
Member

LGTM2

@addyosmani addyosmani merged commit 28f9a2c into v3 Oct 14, 2017
@addyosmani addyosmani deleted the v3-precache-route branch October 14, 2017 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants