Skip to content

Add URLPatternList #166

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.

Sign up for GitHub

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

Open
lucacasonato wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add URLPatternList #166

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
222cdd6
Add URLPatternList
lucacasonato May 3, 2022
5357b1c
simplify
lucacasonato May 5, 2022
14086ee
review comments
lucacasonato May 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
simplify
  • Loading branch information
@lucacasonato
lucacasonato committed May 5, 2022
commit 5357b1cd2d3ff9ff0eb7a1dce786425bdddc82d1
41 changes: 13 additions & 28 deletions spec.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,21 +815,15 @@ To <dfn>compute protocol matches a special scheme flag</dfn> given a [=construct
<xmp class="idl">
[Exposed=(Window,Worker)]
interface URLPatternList {
constructor(sequence<URLPatternListEntry> entries);
constructor(sequence<URLPattern> patterns);

USVString? test(optional URLPatternInput input = {}, optional USVString baseURL);
boolean test(optional URLPatternInput input = {}, optional USVString baseURL);

URLPatternListResult? exec(optional URLPatternInput input = {}, optional USVString baseURL);
};

dictionary URLPatternListEntry {
required (USVString or URLPattern) pattern;
required USVString key;
};

dictionary URLPatternListResult {
URLPatternResult match;
USVString key;
dictionary URLPatternListResult : URLPatternResult {
URLPattern pattern;
Copy link

@posva posva Jun 14, 2023

Choose a reason for hiding this comment

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

I don't know how it works internally but shouldn't it contain the match result as well? Since the point is to pick a pattern to match and it's likely doing some work behind the scenes

Copy link

@bathos bathos Jun 15, 2023

Choose a reason for hiding this comment

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

It does — the : URLPatternResult part means this dictionary extends URLPatternResult.

Copy link

@posva posva Jun 15, 2023

Choose a reason for hiding this comment

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

Aaah i didn’t see it. (Not familiar with this spec language 😂)

};
</xmp>

Expand All @@ -843,17 +837,12 @@ implementation.

Each {{URLPatternList}} has an associated <dfn for=URLPatternList>pattern list</dfn>, a [=list=] of {{URLPattern}}s.

Each {{URLPatternList}} has an associated <dfn for=URLPatternList>key list</dfn>, a [=list=] of [=string=]s.

<div algorithm>
The <dfn constructor for=URLPatternList lt="URLPatternList(entries)">new URLPattern(|entries|)</dfn> constructor steps are:
The <dfn constructor for=URLPatternList lt="URLPatternList(patterns)">new URLPatternList(|patterns|)</dfn> constructor steps are:

1. [=list/For each=] |entry| in |entries|, run the following steps:
1. [=list/Append=] the |entry|'s {{URLPatternListEntry/pattern}} to [=this=]'s [=URLPatternList/pattern list=].
1. Let |key| be |entry|'s {{URLPatternListEntry/key}}.
1. If |key| is the empty string, throw a {{TypeError}}.
1. If [=this=]'s [=URLPatternList/key list=] [=list/contains=] |key|, throw a {{TypeError}}.
1. [=list/Append=] |key| to [=this=]'s [=URLPatternList/key list=].
1. [=list/For each=] |pattern| in |patterns|, run the following steps:
Copy link

@posva posva Jun 14, 2023

Choose a reason for hiding this comment

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

Doesn't this imply that the pattern list is traversed as an array and is potentially slower? #30 (comment)

Copy link

@bathos bathos Jun 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The iteration steps here and elsewhere don’t appear to be observable, so engines can potentially optimize them however they see fit, right? Spec steps usually aim for the simplest and clearest explanation of observable behaviors, not the most efficient ones (because observable behaviors are the only things they can actually specify, further complexity is noise).

Copy link

@posva posva Jun 15, 2023

Choose a reason for hiding this comment

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

I see, thanks for the explanation!

1. If [=this=]'s [=URLPatternList/pattern list=] [=list/contains=] |pattern|, throw a {{TypeError}}.
1. [=list/Append=] the |pattern| to [=this=]'s [=URLPatternList/pattern list=].
1. If [=this=]'s [=URLPatternList/pattern list=] is empty, throw a {{TypeError}}.

Issue: The patterns need to be in a "least -> most significant" sort order so
Expand All @@ -865,24 +854,20 @@ Each {{URLPatternList}} has an associated <dfn for=URLPatternList>key list</dfn>
<div>
The <dfn method for=URLPatternList>test(|input|, |baseURL|)</dfn> method steps are:

1. Let |i| be 0.
1. [=list/For each=] |pattern| in [=this=]'s [=URLPatternList/pattern list=], run the following steps:
1. Let |result| be the result of [=match=] given |pattern|, |input|, and |baseURL| if given.
1. If |result| is null, [=continue=].
1. Return [=this=]'s [=URLPatternList/key list=][|i|].
1. Return null.
1. Return true.
1. Return false.
</div>

<div>
The <dfn method for=URLPatternList>match(|input|, |baseURL|)</dfn> method steps are:

1. Let |i| be 0.
1. [=list/For each=] |pattern| in [=this=]'s [=URLPatternList/pattern list=], run the following steps:
1. Let |match| be the result of [=match=] given |pattern|, |input|, and |baseURL| if given.
1. If |match| is null, [=continue=].
1. Let |result| be a new {{URLPatternListResult}} dictionary.
1. Set |result|'s {{URLPatternListResult/match}} to |match|.
1. Set |result|'s {{URLPatternListResult/key}} to [=this=]'s [=URLPatternList/key list=][|i|].
1. Let |result| be the result of [=match=] given |pattern|, |input|, and |baseURL| if given.
1. If |result| is null, [=continue=].
1. Set |result|'s {{URLPatternListResult/pattern}} to |pattern|.
1. Return |result|.
1. Return null.
</div>
Expand Down