-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Add support for the duplicate named capturing groups proposal #14805
Add support for the duplicate named capturing groups proposal #14805
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52849/ |
Sweet, LGTM!
Yeah, I'm still catching up on stuff from the meeting, sorry. |
ed1e5e3
to
598e911
Compare
) { | ||
// If we only transformed named groups because we want to transform duplicates | ||
// but there are no duplicates, re-transform the regexp without transforming | ||
// named groups. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can regexpu-core
support transforming duplicate named capture groups only? I think it would be useful even out of Babel's scope. And it benefits us, too: we don't have to re-run regexpu-core
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If transform (?<a>)|(?<a>)(?<b>)
to ()|()(?<b>)
and then add an a
property to the .groups
object in our helper, we get Object.keys(groups) == ["b", "a"]
instead of ["a", "b"]
🙁
I can add a check on the regexp before calling regexpu-core that skips the transform if we are sure that there are no duplicates.
Does this plugin need to be added to |
Yes! Thanks for the reminder. |
84b8963
to
5929d75
Compare
Transform plugin for https://github.com/tc39/proposal-duplicate-named-capturing-groups, that reached consensus for Stage 3 in the July meeting (@bakkot the readme needs to be updated!)