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.

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

Jump to bottom

Inconsistent ordering for regex named capture groups in v12.11.0 #29878

Closed
KodiVerse opened this issue Oct 7, 2019 · 4 comments
Closed

Inconsistent ordering for regex named capture groups in v12.11.0 #29878

KodiVerse opened this issue Oct 7, 2019 · 4 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@KodiVerse
Copy link

node:v12.11.0

There is inconsistent ordering for named capture groups from v12.10.0 to v12.11.0. It appears that versions that came before v12.11.0 ordered the keys by the way they were ordered in the regular expression. v12.11.0 seems to sort the keys.

const r = /(?<BKey>.+)\s(?<AKey>.+)/;
const s = 'example string';
// node v12.10.0
r.exec(s).groups;
// { "BKey": "example", "AKey": "string" }
// node v12.11.0
r.exec(s).groups;
// { "AKey": "string", "BKey": "example" }
@addaleax
Copy link
Member

addaleax commented Oct 7, 2019

Interesting – my understand of the spec would indicate to me that the former behaviour is correct: https://tc39.es/ecma262/#sec-regexpbuiltinexec

/cc @nodejs/v8

@addaleax addaleax added the v8 engine Issues and PRs related to the V8 dependency. label Oct 7, 2019
@schuay
Copy link

schuay commented Oct 8, 2019

Filed V8 bug https://crbug.com/v8/9822.

@schuay
Copy link

schuay commented Oct 16, 2019

Fixed upstream in https://crrev.com/c/1864829.

targos added a commit to targos/node that referenced this issue Oct 22, 2019
@targos
deps: V8: cherry-pick ed40ab1
232697f 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: nodejs#29878
@targos targos mentioned this issue Oct 22, 2019
Closed
@targos
Copy link
Member

targos commented Oct 22, 2019

Backport: #30064

@targos targos closed this as completed in d53dd8b Oct 24, 2019
targos added a commit that referenced this issue Oct 28, 2019
@targos
deps: V8: cherry-pick ed40ab1
0d9ae1b 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: #29878

PR-URL: #30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
targos added a commit that referenced this issue Nov 8, 2019
@targos
deps: V8: cherry-pick ed40ab1
d78978f 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: #29878

PR-URL: #30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
targos added a commit that referenced this issue Nov 10, 2019
@targos
deps: V8: cherry-pick ed40ab1
b9f9745 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: #29878

PR-URL: #30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
targos added a commit that referenced this issue Nov 11, 2019
@targos
deps: V8: cherry-pick ed40ab1
d2756fd 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: #29878

PR-URL: #30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
targos added a commit to targos/node that referenced this issue Dec 5, 2019
@targos
deps: V8: cherry-pick ed40ab1
ba5d836 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: nodejs#29878

PR-URL: nodejs#30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
@KodiVerse KodiVerse mentioned this issue Jan 6, 2020
Closed
MylesBorins pushed a commit to targos/node that referenced this issue Jan 7, 2020
@targos @MylesBorins
deps: V8: cherry-pick ed40ab1
aa215bf 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: nodejs#29878

PR-URL: nodejs#30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
MylesBorins pushed a commit that referenced this issue Jan 8, 2020
@targos @MylesBorins
deps: V8: cherry-pick ed40ab1
553afa9 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: #29878

Backport-PR-URL: #30109
PR-URL: #30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
@targos @BethGriggs
deps: V8: cherry-pick ed40ab1
98dfe27 
Original commit message:

    [regexp] Fix the order of named captures on the groups object

    Named capture properties on the groups object should be ordered by the
    capture index (and not alpha-sorted). This was accidentally broken in
    https://crrev.com/c/1687413.

    Bug: v8:9822,v8:9423
    Change-Id: Iac6f866f077a1b7ce557ba47e8ba5d7e7014b3ce
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864829
    Auto-Submit: Jakob Gruber <[email protected]>
    Reviewed-by: Peter Marshall <[email protected]>
    Commit-Queue: Peter Marshall <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64306}

Refs: v8/v8@ed40ab1
Fixes: #29878

Backport-PR-URL: #30109
PR-URL: #30064
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

deps: V8: cherry-pick ed40ab1 targos/node
4 participants
@schuay @addaleax @targos @KodiVerse