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

Rename all Plugin classes to be package-specific #2187

Merged
merged 1 commit into from
Aug 15, 2019

Conversation

philipwalton
Copy link
Member

R: @jeffposnick @philipwalton

Fixes #2072

@jeffposnick I opted not to include any kind of deprecation warning or fallback. Let me know if you think that's needed as I guess this might require a lot of people to change their code.

Also, add a generatePayload option to the BroadcastCacheUpdate instance
that allows users to customize the payload sent.
@jeffposnick
Copy link
Contributor

In terms of a potential fallback, would that entail changing each package's index.ts so that both PackageNamePlugin and Plugin were exported, with both names referencing the same underlying class?

That... doesn't sound too bad, and I'm assuming wouldn't result in a meaningful change to bundle sizes. But I guess it might lead to confusion as to which name to use if developers see both listed in an IDE's autocomplete (assuming they're importing from the index and not a named module).

I'm probably in favor of helping folks who are still using workbox-sw not have to change things, but I could be convinced otherwise.

@workbox-pr-bot
Copy link
Collaborator

PR-Bot Size Plugin

Changed File Sizes

File Before After Change GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js 3.86 KB 3.87 KB +0% 1.60 KB
packages/workbox-broadcast-update/build/workbox-broadcast-update.prod.js 1.01 KB 1.02 KB +1% 578 B
packages/workbox-cacheable-response/build/workbox-cacheable-response.prod.js 594 B 611 B +3% 357 B
packages/workbox-expiration/build/workbox-expiration.prod.js 2.94 KB 2.95 KB +0% 1.27 KB
packages/workbox-google-analytics/build/workbox-offline-ga.prod.js 1.95 KB 1.97 KB +1% 916 B
packages/workbox-range-requests/build/workbox-range-requests.prod.js 1.57 KB 1.58 KB +1% 801 B

New Files

No new files have been added.

All File Sizes

View Table
File Before After Change GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js 3.86 KB 3.87 KB +0% 1.60 KB
packages/workbox-broadcast-update/build/workbox-broadcast-update.prod.js 1.01 KB 1.02 KB +1% 578 B
packages/workbox-build/build/_types.js 41 B 41 B 0% 61 B
packages/workbox-build/build/index.js 3.30 KB 3.30 KB 0% 1.31 KB
packages/workbox-cacheable-response/build/workbox-cacheable-response.prod.js 594 B 611 B +3% 357 B
packages/workbox-cli/build/app.js 4.16 KB 4.16 KB 0% 1.64 KB
packages/workbox-cli/build/bin.js 940 B 940 B 0% 502 B
packages/workbox-core/build/workbox-core.prod.js 5.94 KB 5.94 KB 0% 2.46 KB
packages/workbox-expiration/build/workbox-expiration.prod.js 2.94 KB 2.95 KB +0% 1.27 KB
packages/workbox-google-analytics/build/workbox-offline-ga.prod.js 1.95 KB 1.97 KB +1% 916 B
packages/workbox-navigation-preload/build/workbox-navigation-preload.prod.js 660 B 660 B 0% 323 B
packages/workbox-precaching/build/workbox-precaching.prod.js 4.99 KB 4.99 KB 0% 1.94 KB
packages/workbox-range-requests/build/workbox-range-requests.prod.js 1.57 KB 1.58 KB +1% 801 B
packages/workbox-routing/build/workbox-routing.prod.js 3.09 KB 3.09 KB 0% 1.36 KB
packages/workbox-strategies/build/workbox-strategies.prod.js 4.10 KB 4.10 KB 0% 1.13 KB
packages/workbox-streams/build/workbox-streams.prod.js 1.46 KB 1.46 KB 0% 721 B
packages/workbox-sw/build/workbox-sw.js 1.34 KB 1.34 KB 0% 748 B
packages/workbox-webpack-plugin/build/generate-sw.js 4.24 KB 4.24 KB 0% 1.62 KB
packages/workbox-webpack-plugin/build/index.js 349 B 349 B 0% 255 B
packages/workbox-webpack-plugin/build/inject-manifest.js 5.12 KB 5.12 KB 0% 1.77 KB
packages/workbox-window/build/workbox-window.dev.umd.js 40.62 KB 40.62 KB 0% 8.92 KB
packages/workbox-window/build/workbox-window.prod.umd.js 4.48 KB 4.48 KB 0% 1.83 KB

Workbox Aggregate Size Plugin

3.5KB gzip'ed (23% of limit)
7.87KB uncompressed

@philipwalton
Copy link
Member Author

In terms of a potential fallback, would that entail changing each package's index.ts so that both PackageNamePlugin and Plugin were exported, with both names referencing the same underlying class?

If we want to go deprecation route, I think it be better to make Plugin classes that extend the renamed classes, log a warning, and then call super(). That would be a bit more code, but then at least the code would continue to work and there'd be a warning.

@jeffposnick
Copy link
Contributor

That's more overhead than I was hoping for. We can just go with the approach in this PR (and make sure that we update our docs/samples).

@philipwalton
Copy link
Member Author

SG, if we hear feedback we can always add later.

@philipwalton philipwalton merged commit ccc77b9 into master Aug 15, 2019
@philipwalton philipwalton deleted the rename-plugin-classes branch August 15, 2019 22:01
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.

Rename Plugin classes to include the module name
3 participants