Skip to content

fix: Chaining nullish coalescing operators output size regression #17061

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
Merged
nicolo-ribaudo merged 1 commit into from
Jan 13, 2025
Merged

fix: Chaining nullish coalescing operators output size regression #17061

nicolo-ribaudo merged 1 commit into from
Jan 13, 2025

Conversation

@liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Jan 12, 2025

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

Ref: #16612

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories i: regression Spec: Nullish Coalescing Operator labels Jan 12, 2025
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 12, 2025

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

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jan 13, 2025

I was going to suggest to special case this, but probably having a potentially null this is so rare that it doesn't actually matter.

@JLHwung
Copy link
Contributor

JLHwung commented Jan 13, 2025

I was going to suggest to special case this, but probably having a potentially null this is so rare that it doesn't actually matter.

Agree. It feels like we could have used scope.isStatic(node.left) here, which covers this and other stuffs. As is shown in this PR, we can use a weaker version of isStatic since we don't require the binding to be constant.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jan 13, 2025

Releasing this as-is for now, to fix the output size regression.

@nicolo-ribaudo nicolo-ribaudo merged commit 4408aa6 into babel:main Jan 13, 2025
55 checks passed
@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 Apr 15, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JLHwung JLHwung JLHwung approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees

No one assigned

Labels

i: regression 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 Spec: Nullish Coalescing Operator

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Chaining nullish coalescing operators causes the output size to increase exponentially

4 participants

@liuxingbaoyu @babel-bot @nicolo-ribaudo @JLHwung