fix(optimizer): skip props destructuring on destructured-param reassign (#8638)#8647
Merged
wmertens merged 7 commits intoMay 20, 2026
Merged
Conversation
… reassigned
The props-destructuring pass rewrites identifier *reads* of a destructured
parameter to `_rawProps.<name>`, but does not rewrite assignment LHS. If
the body reassigns a destructured binding (via `=`, `??=`/compound assign,
`++`/`--`, or destructuring-assignment LHS like `({foo} = …)` and
`[foo] = …`), the LHS was left as a now-undeclared identifier — silently
miscompiling the function and producing a strict-mode `ReferenceError` at
runtime. In some cases a downstream simplifier even dropped the assignment
entirely with no error.
Refuse the transform whenever the body reassigns any destructured param.
This is safe because Qwik component props are read-only, so reassigning
one is semantically meaningless — affected code is either a non-component
helper that happens to share the destructuring shape, or a bug in user
code that should be surfaced rather than silently miscompiled.
Adds regression tests covering template-literal, string-literal,
nullish-assign, update-expression (`++`), object-destructuring-assignment
(`({foo} = …)`), and array-destructuring-assignment (`[foo] = …`)
reassignment forms.
…n DocumentHead
🦋 Changeset detectedLatest commit: f1cbd63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
…structure-reassign
wmertens
requested changes
May 20, 2026
Member
wmertens
left a comment
wmertens
left a comment
There was a problem hiding this comment.
Thank you for the PR; however, can you make the snapshots way more compact (testing several cases in a single test), and can you also add specific code that checks the result so we don't have to rely only on the snapshot (there are other tests that check this too)
We're overhauling the optimizer and adding all these snapshots is a maintenance burden.
…ots into direct assertions
@qwik.dev/core
@qwik.dev/router
eslint-plugin-qwik
create-qwik
@qwik.dev/optimizer
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is it?
Description
transform_props_destructuringmis-rewrites plain helper arrows, breaking dev SSR and production #8638.The optimizer's
transform_props_destructuringpass was rewritingcomponent$bodies that destructure props and then reassign one of the destructured names — including reassignments insidefor-of/for-inheads — even though the rewrite is unsound in that case. The resulting code threw aReferenceErrorat SSR time, which surfaced as a dev-SSR hang (request never resolved) or a 500 in production.Two-commit fix in the Rust optimizer:
ReassignFindernow detectsAssignExpr/UpdateExprwrites to a destructured name and refuses the rewrite.for-of/for-inloop heads (for (x of …),for ({ x } of …)).Coverage:
packages/optimizer/core/src/test.rsfor both the base case and the for-of/for-in variants.e2e/qwik-e2e/tests/qwikrouter/issue8638-document-head-helper.e2e.ts, using aDocumentHeadhelper that exercises the original bug pattern end-to-end (Vite → optimizer → SSR → emitted HTML). The e2e asserts: no dev-SSR hang, nopageerror, and theog:imagemeta is emitted with the helper's default (guards against silent dead-code drop).Sanity-checked by temporarily reverting the optimizer change, rebuilding WASM/NAPI, and confirming the e2e fails on the
og:imageassertion.Checklist
pnpm change