Fix router rule initialization #3
Conversation
|
Since the Handle Fetch algorithm checks the existence of the rule with set/unset ("14. Else if registration’s active worker’s list of router rules is set:"), I suppose it fine to leave as-is. Ah, to run "Let |routerRules| be a copy of |serviceWorker|'s [=list of router rules=]." in https://github.com/yoshisatoyanagisawa/ServiceWorker/pull/2/files as-is, it need to be initialized with an empty set. How about initializing the variable there? |
I suppose that |
|
Sounds good to me. |
| @@ -3107,7 +3107,7 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/ | |||
| 1. Assert: |request|'s [=request/destination=] is not "<code>serviceworker</code>". | |||
| 1. If |request|'s [=request/destination=] is either "<code>embed</code>" or "<code>object</code>", then: | |||
| 1. Return null. | |||
| 1. Else if |registration|'s <a>active worker</a>'s [=service worker/list of router rules=] is set: | |||
| 1. Else if |registration|'s <a>active worker</a>'s [=service worker/list of router rules=] is [=list/is not empty=]: | |||
There was a problem hiding this comment.
I think it is more elegant to remove this condition entirely, since if the list is empty, "Get Router Source" will return null, so the substep will not trigger.
There was a problem hiding this comment.
Since I do not think it is a good idea to ask the intern to work on this after the leave, let me take over the part. I am going to merge this as-is, and will address that in the follow up PR, which might come next year.
There was a problem hiding this comment.
Anyway, thank you for your comment.
There was a problem hiding this comment.
It is totally OK for me to do such small things as an ooen source contributor, but thanks for your consideration.
Please feel free to ask me if there are any inconvenience due to my work!
yoshisatoyanagisawa
merged commit a44f9a4
into
yoshisatoyanagisawa:static_routing_api
The
list of router rulesofservice workeris currently initialized as if it is a flag, but it is actually a list.This PR fix this bug.
Preview | Diff