fix: prevent framework documentation page from freezing on link hover - #1010
fix: prevent framework documentation page from freezing on link hover#1010Manishkumarbingi wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe router install documentation path changes from a framework-specific path to ChangesRouter install path
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Router framework documentation links now use the canonical quick-start URL, avoiding the prior framework-prefixed redirect path. Focused coverage verifies the generated links across supported frameworks, with no remaining merge-readiness risk identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
@tannerlinsley Please have a look at this PR , If not required feel free to close this PR. |
|
I reproduced the production freeze today, but preload={false} only avoids the redirect during hover. The card currently builds /router/latest/docs/framework/{framework}/quick-start from the Router installPath. The Router library beforeLoad then always redirects every non-example framework guide to /router/latest/docs/quick-start. Opening that canonical path directly is responsive and lands on the same page. Please point the Router installPath at quick-start instead of disabling preloading on both links, add focused coverage for the emitted Router install href, and update the PR to close #1001. This preserves intent preloading and removes the unnecessary client redirect. I also checked #990 and #866 so this direction keeps the earlier quick-start correction while avoiding the redirect path that caused the same class of freeze. |
… disabling preload
75e060c to
c4f8ab5
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Thanks for reproducing this and finding the actual cause — you're right, the redirect was the real issue. Traced it through FrameworkCard → getFrameworkDocsPath → the base router object in libraries.ts, which had installPath: 'framework/$framework/quick-start'. Confirmed against the router repo's docs that there's no per-framework quick-start.md (only the canonical one), so that path always triggered the loader redirect. Pushed a fix pointing installPath at quick-start, reverted the preload={false} change, and added a focused unit test on the emitted install href for both frameworks. PR description updated to close #1001. |
Summary
Root cause: Router's Quick Start doc is canonical and not split per framework, so installPath: 'framework/$framework/quick-start' pointed FrameworkCard's link at a doc that doesn't exist. The docs loader redirected away from it, and that redirect firing on hover/preload is what froze the page.
Fix: point installPath at the canonical quick-start doc instead of disabling preload. Added focused test coverage on the emitted install href per review feedback.
Closes #1001
Summary by CodeRabbit
Bug Fixes
Tests