Skip to content

Do not mark OptionalMemberExpresion as LVal #17432

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 4 commits into from
Jul 12, 2025
Merged

Do not mark OptionalMemberExpresion as LVal #17432

nicolo-ribaudo merged 4 commits into from
Jul 12, 2025

Conversation

@JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jul 11, 2025 •
edited by nicolo-ribaudo
Loading

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

In this PR we remove the OptionalMemberExpression from the LVal alias. The left of the AssignmentExpression still allows OptionalMemberExpression, so the builder usage and the internal typings are generally not affected.

This PR partially reverts #17391.

@JLHwung JLHwung added the PR: Revert ↩️ A type of pull request used for our changelog categories label Jul 11, 2025
@babel-bot
Copy link
Collaborator

babel-bot commented Jul 11, 2025

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jul 11, 2025

Open in StackBlitz

commit: 3bdd2e7

nicolo-ribaudo
nicolo-ribaudo approved these changes Jul 12, 2025
View reviewed changes
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.

Releasing this soon!

@nicolo-ribaudo nicolo-ribaudo changed the title Reverts OptionalMemberExpression LVal alias Do not mark OptionalMemberExpresion as LVal Jul 12, 2025
@nicolo-ribaudo nicolo-ribaudo merged commit 8b2e9b4 into babel:main Jul 12, 2025
60 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the revert-optional-member-expression-lval branch July 12, 2025 08:03
@aldobarr
Copy link

aldobarr commented Jul 12, 2025

@JLHwung I just tested 7.28.1 and while the issue I reported in #17434 appears to be resolved, my builds are still failing. They succeed in 7.27.* though. My project is open, you can try it out if it'd help you at all. Here's what I see now on 7.28.1:

error during build:
Cannot add property 0, object is not extensible
    at Array.push (<anonymous>)
    at ConditionalExpression.getLiteralValueAtPath (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:12270:45)
    at file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:5064:30
    at EntityPathTracker.withTrackedEntityAtPath (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:2014:24)
    at LocalVariable.getLiteralValueAtPath (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:5062:33)
    at Identifier.getLiteralValueAtPath (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:5207:48)
    at MemberExpression.getDynamicPropertyKey (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:7636:41)
    at MemberExpression.hasAccessEffect (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:7652:65)
    at MemberExpression.hasEffects (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:7433:18)
    at AssignmentExpression.hasEffects (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:11567:23)
    at ExpressionStatement.hasEffects (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:2809:28)
    at ExpressionStatement.shouldBeIncluded (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:2928:62)
    at ExpressionStatement.shouldBeIncluded (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:6470:22)
    at BlockStatement.include (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:6518:56)
    at includeLoopBody (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:12451:10)
    at WhileStatement.include (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:15421:9)
    at BlockStatement.include (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:6519:26)
    at FunctionDeclaration.include (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:6715:19)
    at FunctionDeclaration.include (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:6809:15)
    at Graph.includeStatements (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:22548:36)
    at Graph.build (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:22487:14)
    at async file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:23172:13
    at async catchUnfinishedHookActions (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:22643:16)
    at async rollupInternal (file:///home/workspaces/node_modules/rollup/dist/es/shared/node-entry.js:23167:5)
    at async buildEnvironment (file:///home/workspaces/node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:46206:14)
    at async Object.defaultBuildApp [as buildApp] (file:///home/workspaces/node_modules/vite/dist/node/chunks/dep-DBxKXgDP.js:46684:5)
    at async CAC.<anonymous> (file:///home/workspaces/node_modules/vite/dist/node/cli.js:863:7)

@chrisbbreuer chrisbbreuer mentioned this pull request Aug 22, 2025
Open
1 task
@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 Oct 12, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

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

@liuxingbaoyu liuxingbaoyu liuxingbaoyu approved these changes

Assignees

No one assigned

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: types PR: Revert ↩️ A type of pull request used for our changelog categories

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7.28.0 Regression on TypeScript transform when passing references OptionalMemberExpressions are being reported as LVal's

5 participants

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