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

consider throwing if a single component pattern has duplicate named groups #96

Closed
wanderview opened this issue Aug 16, 2021 · 0 comments

Comments

@wanderview
Copy link
Member

Currently, its possible to do this:

let p = new URLPattern({ pathname: '/:id/:id' });
let r = p.exec({ pathname: '/foo/bar' });

console.log(r.pathname.groups.id); // logs 'bar'

I think this behavior is a bit surprising and probably not what people would expect. Its possible we could support this with some different behaviors (require groups to match the same value, create a sublist of values, etc), but for now I'd like to throw on duplicate names to prevent folks from depending on this unintended behavior.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 17, 2021
As discussed in:

whatwg/urlpattern#96

It was previously possible to use duplicate group names in a single
pattern string.  The value matched for the last group with the same
name would "win".

This was unintended behavior.  For now we should just throw on
duplicate identifier names to avoid developers coming to depend on
this behavior.

Fixed: 1240151
Change-Id: Iecccf8e3a4606917f6b58a627f86819ee0f774d4
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 17, 2021
As discussed in:

whatwg/urlpattern#96

It was previously possible to use duplicate group names in a single
pattern string.  The value matched for the last group with the same
name would "win".

This was unintended behavior.  For now we should just throw on
duplicate identifier names to avoid developers coming to depend on
this behavior.

Fixed: 1240151
Change-Id: Iecccf8e3a4606917f6b58a627f86819ee0f774d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3099258
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#912691}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 17, 2021
As discussed in:

whatwg/urlpattern#96

It was previously possible to use duplicate group names in a single
pattern string.  The value matched for the last group with the same
name would "win".

This was unintended behavior.  For now we should just throw on
duplicate identifier names to avoid developers coming to depend on
this behavior.

Fixed: 1240151
Change-Id: Iecccf8e3a4606917f6b58a627f86819ee0f774d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3099258
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#912691}
pull bot pushed a commit to ZSX-JOJO/chromium that referenced this issue Aug 18, 2021
As discussed in:

whatwg/urlpattern#96

It was previously possible to use duplicate group names in a single
pattern string.  The value matched for the last group with the same
name would "win".

This was unintended behavior.  For now we should just throw on
duplicate identifier names to avoid developers coming to depend on
this behavior.

Fixed: 1240151
Change-Id: Iecccf8e3a4606917f6b58a627f86819ee0f774d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3099258
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#912691}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 3, 2021
…es., a=testonly

Automatic update from web-platform-tests
URLPattern: Throw on duplicate group names.

As discussed in:

whatwg/urlpattern#96

It was previously possible to use duplicate group names in a single
pattern string.  The value matched for the last group with the same
name would "win".

This was unintended behavior.  For now we should just throw on
duplicate identifier names to avoid developers coming to depend on
this behavior.

Fixed: 1240151
Change-Id: Iecccf8e3a4606917f6b58a627f86819ee0f774d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3099258
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#912691}

--

wpt-commits: dd1bd4510455460fbf7086faf340fb86f685fd5f
wpt-pr: 30053
spinda pushed a commit to PLSysSec/cachet-firefox that referenced this issue Sep 8, 2021
…es., a=testonly

Automatic update from web-platform-tests
URLPattern: Throw on duplicate group names.

As discussed in:

whatwg/urlpattern#96

It was previously possible to use duplicate group names in a single
pattern string.  The value matched for the last group with the same
name would "win".

This was unintended behavior.  For now we should just throw on
duplicate identifier names to avoid developers coming to depend on
this behavior.

Fixed: 1240151
Change-Id: Iecccf8e3a4606917f6b58a627f86819ee0f774d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3099258
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#912691}

--

wpt-commits: dd1bd4510455460fbf7086faf340fb86f685fd5f
wpt-pr: 30053
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
As discussed in:

whatwg/urlpattern#96

It was previously possible to use duplicate group names in a single
pattern string.  The value matched for the last group with the same
name would "win".

This was unintended behavior.  For now we should just throw on
duplicate identifier names to avoid developers coming to depend on
this behavior.

Fixed: 1240151
Change-Id: Iecccf8e3a4606917f6b58a627f86819ee0f774d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3099258
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#912691}
NOKEYCHECK=True
GitOrigin-RevId: 003c5485372aa2b54cde419f7dc20dd5ec03de42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant