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

Handle multiple named groups in wrapRegExp replace() #15090

Merged
merged 1 commit into from Oct 29, 2022
Merged

Handle multiple named groups in wrapRegExp replace() #15090

merged 1 commit into from Oct 29, 2022

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Oct 28, 2022

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

Previously, the [Symbol.replace] implementation in BabelRegExp assumed that the groups object property values were only numbers. With the duplicate named capturing groups plugin, they can be arrays of numbers as well.

In the case of a duplicate named capturing group, the replacement pattern $<name> can be replaced by the concatenation of the pattern indices that have that name: e.g., $1$2. The bug was that the arrays got stringified in replacement patterns, leading to replacement patterns like $1,2.

@babel-bot
Copy link
Collaborator

babel-bot commented Oct 28, 2022

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

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Thank you!

packages/babel-helpers/src/helpers/wrapRegExp.js Outdated Show resolved Hide resolved
@nicolo-ribaudo nicolo-ribaudo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Oct 28, 2022
Previously, the [Symbol.replace] implementation in BabelRegExp assumed
that the groups object property values were only numbers. With the
duplicate named capturing groups plugin, they can be arrays of numbers
as well.

In the case of a duplicate named capturing group, the replacement
pattern $<name> can be replaced by the concatenation of the pattern
indices that have that name: e.g., $1$2.
@ptomato ptomato requested review from JLHwung and removed request for nicolo-ribaudo October 28, 2022 22:50
@nicolo-ribaudo nicolo-ribaudo merged commit 19090d4 into babel:main Oct 29, 2022
@ptomato ptomato deleted the dncg-fix-replace branch October 29, 2022 20:49
@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 Jan 29, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 29, 2023
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: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants