fix(data-loaders): ignore leaked loader context in components - #2727
fix(data-loaders): ignore leaked loader context in components#2727posva wants to merge 2 commits into
Conversation
A still-pending lazy nested loader leaves the module-level loader
context set to its own entry. When a component renders during that
window, `useDataLoader()` in `setup()` reads the stale context as its
parent, which re-runs the parent loader, nests a loader under itself
("… has itself as parent") and can spin into an infinite render loop.
In a component, loaders are always used at the root, so drop any active
context unless we are synchronously inside a loader function (genuine
nesting). A small depth counter (`runInsideLoaderFn`/`isInsideLoaderFn`)
keeps real nested calls working even when triggered from a component.
Closes #2684
✅ Deploy Preview for vue-router canceled.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2727 +/- ##
==========================================
+ Coverage 86.43% 87.23% +0.79%
==========================================
Files 91 93 +2
Lines 10395 10918 +523
Branches 2426 2501 +75
==========================================
+ Hits 8985 9524 +539
+ Misses 1400 1388 -12
+ Partials 10 6 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for looking into this issue! I have prepared another reproduction, this time with more loaders If you open console and check logs, you can see that sometimes context is lost, as evident by messages about using I also propose my solution to this, you can find PR here |
Ports the reload regression test from #2757 and adds coverage for reloading deeply nested loaders. Runs for both the basic and the Pinia Colada loaders. Co-authored-by: Ilia Bolotov <Ilia_bolotov@epam.com>
|
@Truevoly thanks, your approach broke other tests so I copied over the test to here |
2 participants
Closes #2684
Problem
When all loaders on a route are lazy, navigation completes while the loaders are still pending. A nested lazy loader (e.g.
childthatawaitsparentand then keeps loading) leaves the module-level loader current context pointing at its own entry. If a component renders during that window,useDataLoader()called insetup()reads that stale context as its parent, which:"…" has itself as parent),The context global is set around a loader's execution and restored on
.finally; lazy loaders restore it after navigation, racing with component rendering.Fix
In
useDataLoader, when invoked from inside a component (getCurrentInstance()) and not synchronously within a loader function, drop the ambient context — in a component, loaders are always used at the root. A small depth counter (runInsideLoaderFn/isInsideLoaderFninutils.ts) wraps the actual loader-function call so genuinely nested loaders still nest correctly, even when first triggered synchronously from a component. Applied to bothdefineBasicLoaderanddefineColadaLoader.Test
Added a regression test to the shared
tester.ts(runs for basic + Colada): a parent loader that resolves instantly, a child loader that nests it and keeps loading, and parent/child components that use the loaders directly. Written before the fix and confirmed failing first ("has itself as parent"warning + parent loader called twice); passes after.Checks
pnpm test:unit— full suite passes (1629)pnpm test:types