fix(data-loaders): don't re-run instantly resolving parents when nested loaders attach - #2794
fix(data-loaders): don't re-run instantly resolving parents when nested loaders attach#2794lvyanyan wants to merge 1 commit into
Conversation
…ed loaders attach Lazy loaders that resolve instantly get committed before nested loaders run, nulling their pendingTo. When a nested loader then attached itself (via useDataLoader), the pendingTo !== route check wrongly considered the parent stale and forced a refetch, running the parent loader twice (and potentially forever when combined with cached helpers). - reuse the committed entry when it already has data for the current route - propagate an explicit force flag (context 4th slot) so reload() still re-runs nested loaders for consistency fixes vuejs#2684
✅ Deploy Preview for vue-router canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
ChangesNested loader execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change prevents unintended duplicate parent-loader execution while preserving explicit reload behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Router
participant ParentLoader
participant ChildLoader
Router->>ParentLoader: navigate to route
ParentLoader->>ChildLoader: resolve nested loader context
ChildLoader->>ChildLoader: check route data and force state
ChildLoader-->>Router: complete navigation
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes address issue
✨ 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 |
fixes #2684
Context
In #2684, when all loaders are lazy and the parent loader resolves instantly (e.g. a synchronous cache), the parent loader ends up executed twice per navigation — and with a caching helper component the repeated executions can loop forever.
Root cause
Lazy loaders are executed at component setup, after the navigation has finished. With
commit: 'after-load'(the default), a loader that resolves instantly commits immediately, nullingentry.pendingTo. When a nested loader later callsuseParentLoader()inside its own query, theentry.pendingTo !== routecheck seesnulland assumes the entry is stale, forcing a refetch (reload=truewas hardcoded for nested calls) — running the parent loader a second time.What this PR does
entry.to === routeand it has apendingLoad), a nesteduseDataLoader()call reuses the resolved query instead of forcing a refetch.reload()still forces nested loaders to re-run: the force flag travels through the loader context (new optional 4th slot) so nested calls inherit it.Tests
Two new tests reproduce the issue with lazy loaders + instantly resolving parents (one for
defineBasicLoader, one fordefineColadaLoaderwith a cached parent). Both fail without the fix and pass with it. Fullpackages/routersuite: 1689 passing.Co-authored note: diagnosis guided by @posva's comment pinpointing the context reset timing.
Summary by CodeRabbit
New Features
Bug Fixes