fix: detect mutations within assignments expressions #12411
Conversation
This enhances our "variable was mutated" detection to also recognize that `foo` in `[foo[1]] = [..]` was mutated. This allows us to be more granular about detecting mutations to each expressions in legacy mode, which fixes #12169
🦋 Changeset detectedLatest commit: 9b9a268 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| case 'MemberExpression': | ||
| if (include_member_expressions) { | ||
| // Only the `a` from `[a.b[c].d]` is of interest to us here | ||
| const id = object(param); | ||
| if (id) nodes.push(id); | ||
| } | ||
| break; |
There was a problem hiding this comment.
I don't understand what any of this is doing. If we revert all the changes in this file and keep the ones in 2-analyze/index.js, the test still passes. Do we need it? It sets off my spidey senses
There was a problem hiding this comment.
Try running the legacy runtime test suite, you'll see one test failing then
There was a problem hiding this comment.
I figured out what was triggering my spidey senses — this causes mutations and reassignments to be treated the same way, resulting in less efficient code in the [array[1], array[0]] = [array[0], array[1]] case. It's an edge case to be sure but I thought it worthwhile to get to the bottom of it anyway — opened #12429 with a very slightly different approach
This enhances our "variable was mutated" detection to also recognize that
fooin[foo[1]] = [..]was mutated. This allows us to be more granular about detecting mutations to each expressions in legacy mode, which fixes #12169Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.Tests and linting
pnpm testand lint the project withpnpm lint