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

Editorial: Add notice on the _or condition depth. #1714

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yoshisatoyanagisawa
Copy link
Contributor

@yoshisatoyanagisawa yoshisatoyanagisawa commented Apr 30, 2024

In WICG/service-worker-static-routing-api#5 and WICG/service-worker-static-routing-api#6, it was suggested to limit the number of conditions in the or condition syntax, and depth of nested and/or conditions.

This PR adds notice on such limitations.


Preview | Diff

@yoshisatoyanagisawa
Copy link
Contributor Author

@domenic Can I ask you to review?
@sisidovski @azaika I hope this covers what you meant in the github issues. I tried to avoid specific numbers because I wanted to give some flexibility on the way browsers implement this.

@mkruisselbrink
Copy link
Collaborator

It probably would be good to have a (normative) requirement for the minimum nesting level that browser should support? (with corresponding WPT tests)

@yoshisatoyanagisawa
Copy link
Contributor Author

It probably would be good to have a (normative) requirement for the minimum nesting level that browser should support? (with corresponding WPT tests)

If my put myself in the web developers shoes, it is understandable. They should want to know the depth that works on all browsers.

We currently have https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/service-workers/service-worker/tentative/static-router/resources/router-rules.js;l=76;drc=bc4f2163da44000fc411e6aeff490f19c414b495 for the nested conditions. I do not come up with the case more nests are needed.

Let me add such tests to not, and revisit here.

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.

2 participants