-
Notifications
You must be signed in to change notification settings - Fork 818
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 cleanRedirect logic #1678
Fix cleanRedirect logic #1678
Conversation
@@ -17,8 +17,12 @@ | |||
// Stub missing/broken Headers API methods in `service-worker-mock`. | |||
// https://fetch.spec.whatwg.org/#headers-class | |||
class Headers { | |||
constructor(obj = {}) { | |||
this.obj = Object.assign({}, obj); | |||
constructor(init = {}) { |
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.
We've diverged from service-worker-mock
over the years, but this change is roughly https://github.com/pinterest/service-workers/blob/c0eda82a98aa46d9a6fb44a32906fa0c2a4f2b22/packages/service-worker-mock/models/Headers.js#L4-L12
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.
LGTM, do we want to merge this into master to fix the bug more quickly, or do you want to save this for v4?
|
PR-Bot Size PluginChanged File Sizes
New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size Plugin9.38KB gzip'ed (63% of limit) |
R: @philipwalton
While investigating #1677 I realized that there's a bug in the
Request
constructor usage inside the code that "cleans up" redirected responses.The code in this PR just takes the corresponding code from
sw-precache
's implementation, which should work.I'm filing this PR against
next
because I don't think it's an urgent bug to fix, and I'd rather we don't cut any additional v3 releases if we can help it.