fix(qwik): guard undefined vNode in scheduleTask and scheduleEffects#8559
fix(qwik): guard undefined vNode in scheduleTask and scheduleEffects#8559juslintek wants to merge 5 commits intoQwikDev:rolldown-vite-supportfrom
Conversation
When the qwikloader dispatches events asynchronously, the Task's host element ($el$) can be undefined if the container was destroyed during async dispatch or if deserialization produced an incomplete Task object. This causes `markVNodeDirty` to crash with: TypeError: Cannot read properties of undefined (reading 'dirty') Add null checks for `task.$el$` / `consumer.$el$` before calling `markVNodeDirty`, consistent with the guard already present in `wrapped-signal-impl.ts`.
Verify that scheduleTask does not throw when task.$el$ is undefined or when the task itself is not properly deserialized.
🦋 Changeset detectedLatest commit: efa3221 The changes in this PR will be included in the next version bump. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4157811b77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Move vi.mock calls to module level (where Vitest hoists them) and use Object.defineProperty to set _captures per-test. This avoids the closure-over-test-local-variables issue flagged in review. Also adds a positive test verifying markVNodeDirty IS called when task.$el$ is defined.
Varixo
left a comment
Varixo
left a comment
There was a problem hiding this comment.
Hello! Thank you for the PR.
Unfortunately for me it looks more like workaround instead of real solution. Basically task should have always host element. If there is a situation when it doesn't then its already a bug imo. Also the test case should show real example when useTask$ will have no host element.
Additionally the rolldown support branch is a bit behind the v2 branch, so first we need to check if this is fixed.
Hi, thanks for review. Should I attempt a rebase of parent branch with this fix in a separate branch to validate whether it is working or not? Regarding test I will create real example of my current project that I am working with this version of qwik. |
…ing-async-dispatch scenario Addresses review feedback from @Varixo — tests now simulate the actual DomContainer.$destroy$() flow where $getObjectById$ returns undefined after container destruction during async qwikloader dispatch, rather than artificially constructing Tasks with undefined host elements. The tests mock getDomContainer (not qrl-class internals) and exercise the real deserializeCaptures → $getObjectById$ → undefined path that causes the crash in production.
The bug shouldn't be related to rolldown support, so you can just fix it on the build/v2 branch. Thank you for your work! |
2 participants
What is it?
Description
Fixes a runtime crash during hydration where
markVNodeDirtyreceives anundefinedvNode, causing:This crash makes the entire application non-functional — no page renders, no login, all routes are dead.
Root Cause
scheduleTask(inuse-task.ts) andscheduleEffects(inreactive-primitives/utils.ts) passtask.$el$/consumer.$el$tomarkVNodeDirtywithout checking if the host element is defined.task.$el$can beundefinedwhen:dispatchis now async, and if a navigation/destruction happens during anawait, the container's$stateData$is truncated, causing subsequent deserialization to returnundefinedfor VNode references)Other callers of
markVNodeDirtyalready have this guard — for example,wrapped-signal-impl.tschecksif (this.$container$ && this.$hostElement$)before callingmarkVNodeDirty. This PR adds the same defensive pattern to the two unguarded call sites.Changes
packages/qwik/src/core/use/use-task.tstask?.$el$inscheduleTaskbefore callingmarkVNodeDirtypackages/qwik/src/core/reactive-primitives/utils.tsconsumer.$el$in theisTask(consumer)branch ofscheduleEffectsbefore callingmarkVNodeDirtypackages/qwik/src/core/use/use-task.unit.tsscheduleTaskdoes not throw whentask.$el$or_captures[0]isundefined, and one positive test verifyingmarkVNodeDirtyIS called whentask.$el$is definedvi.mockwith per-testObject.definePropertyto avoid the Vitest hoisting trap.changeset/gentle-tasks-guard.md@qwik.dev/coreDocs
No user-facing documentation changes needed —
scheduleTask,scheduleEffects, andmarkVNodeDirtyare internal APIs not exposed in the public docs or GLOSSARY.Checklist
pnpm change