feat(web): implement dynamic Preact re-hydration in ClientLink for SPA navigation - #948
feat(web): implement dynamic Preact re-hydration in ClientLink for SPA navigation#948ubay1 wants to merge 2 commits into
Conversation
…A navigation: Replace hacky legacy DOM query re-initialization functions with native Preact dynamic hydration. Key changes: - Dynamically import the target page's entrypoint module script after swapping #root via innerHTML. - Append a timestamp query parameter to bypass browser ES module evaluation cache, ensuring Preact executes hydrate() on every transition. - Automatically re-attach all synthetic React/Preact event listeners (CodeBox copy buttons, tabs) without storage dependencies. - Remove obsolete legacy functions and translate internal comments to English. - Update web generator documentation (README.md).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview
The default Reviewed by Cursor Bugbot for commit b091c5f. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@ubay1 is attempting to deploy a commit to the OpenJS Foundation Team on Vercel. A member of the Team first needs to authorize it. |
MattIPv4
left a comment
There was a problem hiding this comment.
I will let others weigh in on whether they want to introduce SPA navigation or not, but I just want to be clear, this is an incredibly complex change to solve the position of a scrollbar, and not at all what I suggested as a solution to the issue:
Yes, I'd encourage you to open a PR in doc-kit instead, to have it scroll to the active item on page load
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b091c5f. Configure here.
| const scriptUrl = new URL( | ||
| newScript.getAttribute('src'), | ||
| window.location.origin | ||
| ); |
There was a problem hiding this comment.
Script URL resolved against origin
High Severity
reinitPageFeatures builds the entrypoint URL with window.location.origin as the base. Relative script src values such as ../page.js therefore resolve from the domain root instead of the current page path, so the wrong module is requested (or a 404) whenever the site is not hosted at /. Preact never re-hydrates and interactive controls stop working after SPA navigation.
Reviewed by Cursor Bugbot for commit b091c5f. Configure here.
| * @param {string} url URL | ||
| */ | ||
| const redirect = url => (window.location.href = url); | ||
| const redirect = url => navigate(url); |
There was a problem hiding this comment.
Version switch uses SPA navigate
High Severity
redirect now always calls navigate(), including for the version Select onChange. Version values are absolute {baseURL}/latest-{version}/api{path}.html URLs for different builds. Soft-navigating only swaps #root and leaves the previous page’s import map and assets in <head>, so cross-version navigation loads mismatched modules and breaks the page.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b091c5f. Configure here.
| // 2. Push browser history (skip for popstate – browser already did it) | ||
| if (!isPopState) { | ||
| window.history.pushState({}, '', url); | ||
| } |
There was a problem hiding this comment.
pushState runs outside error handling
Medium Severity
history.pushState runs before the try/catch that falls back to window.location.href. For cross-origin version URLs, pushState throws a SecurityError, so the catch never runs and navigation fails with an unhandled exception instead of a full page load.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b091c5f. Configure here.
avivkeller
left a comment
There was a problem hiding this comment.
I agree with Matt. While I appreciate the effort here (well, the effort of your agent, I suppose), this is extremely complex and fundamentally changes the way we handle links
Yes it's very complicated. I just had the idea and my agent helped me turn it into code. At least for me, these docs are now much easier to use. |
|
If you want to solve the problem, that the sidebar scroll position resets, we don't need to rearchitect the entire project... as I suggested originally, a very simple solution would be for the sidebar to scroll to whichever item is active when the page loads. |
Please disclose AI usage in your PRs: https://ai-coding-assistants-policy.openjsf.org/ |


Summary
This PR introduces the
ClientLinkcomponent andnavigate()utility for thewebgenerator to enable zero-reload Single Page Application (SPA) navigation.This directly solves the issue where navigating between documentation pages resets the sidebar scroll position back to the top, while also addressing the architectural feedback from nodejs/learn#75 (comment) by avoiding browser storage (
localStorage/sessionStorage) entirely.Problem Statement
When users navigate between generated static documentation pages using standard
<a>links:Sidebar Scroll Reset: The browser performs a full page reload on every click. This destroys and rebuilds the DOM, causing the sidebar (
<aside>) scroll position to always jump back to the very top disrupting the user's reading and navigation experience.Solution: Introducing
ClientLinkTo solve the sidebar scroll reset cleanly, we introduce the
ClientLinkcomponent and programmaticnavigate()helper 1. Zero-Reload Navigation with In-Memory Scroll State:ClientLinkintercepts internal link clicks, fetches target HTML in the background, and swaps the#rootcontainer. Before the swap, it savesaside.scrollTopinto a module-scoped JavaScript variable and restores it immediately afterward—keeping the sidebar exactly where the user left it without needinglocalStorage.2. Smooth Crossfades & History: Uses the View Transitions API (
document.startViewTransition()) when available and fully supports browser Back / Forward buttons viapopstate.Usage Guide (How to Use)
1. Using
ClientLinkin Sidebar / Navigation ComponentsImport
ClientLinkand pass it as theasprop to<SideBar>(or use it as a drop-in replacement for standard<a>tags):Verification & Testing
Linting (
npm run lint): Passed (0 errors, 0 warnings).Unit Test Suite (
npm run test): 100% Passed (448 tests across 137 test suites).Test Coverage (
npm run test:coverage): Confirmed no regression in core web generator utilities.Manual Verification: Tested local SPA navigation between pages; confirmed
CodeBoxcopy buttons andCodeTabswork smoothly immediately after page transitions without full reloads.I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run
node --run testand all tests passed.I have check code formatting with
node --run format&node --run lint.I've covered new added functionality with unit tests if necessary.