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

Add support for the duplicate named capturing groups proposal #14805

Merged

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Jul 27, 2022

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Y
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

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!)

@nicolo-ribaudo nicolo-ribaudo added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Jul 27, 2022
@nicolo-ribaudo nicolo-ribaudo added this to the 7.19.0 milestone Jul 27, 2022
@babel-bot
Copy link
Collaborator

babel-bot commented Jul 27, 2022

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52849/

@bakkot
Copy link
Contributor

bakkot commented Jul 27, 2022

Sweet, LGTM!

@bakkot the readme needs to be updated!

Yeah, I'm still catching up on stuff from the meeting, sorry.

@nicolo-ribaudo nicolo-ribaudo added PR: Needs Review A pull request awaiting more approvals PR: Needs Docs labels Aug 16, 2022
) {
// 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.
Copy link
Contributor

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.

Copy link
Member Author

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.

@liuxingbaoyu
Copy link
Member

Does this plugin need to be added to standalone?

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 25, 2022

Yes! Thanks for the reminder.

@nicolo-ribaudo nicolo-ribaudo added PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release and removed PR: Needs Review A pull request awaiting more approvals labels Aug 30, 2022
@nicolo-ribaudo nicolo-ribaudo merged commit 80c6889 into babel:main Sep 2, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the proposal-duplicate-named-groups branch September 2, 2022 16:06
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants