Repository navigation
fix(navigation): make blog transitions reliable - #10
Merged
Merged
Conversation
Reviewer's GuideThe PR hardens blog transitions by coordinating the custom wipe with client-router commits, adding a guarded native-navigation fallback and route loading states, while restoring link prefetching and standardizing server content caching. Sequence diagram for reliable blog route transitionssequenceDiagram
actor User
participant P3RLink
participant P3REffects
participant ClientRouter
participant BlogRoute
participant Browser
User->>P3RLink: click blog link
P3RLink->>P3REffects: dispatch P3RWipeRequest
P3REffects->>ClientRouter: router.push or router.replace
P3REffects->>P3REffects: expand wipe and wait for commit
ClientRouter-->>P3REffects: route commit
P3REffects->>BlogRoute: reveal route
BlogRoute-->>User: show route-loading while content loads
alt client route stalls for 8000ms
P3REffects->>Browser: location.assign(destination)
end
Flow diagram for blog navigation fallbackflowchart TD
A[Blog link clicked] --> B{P3R transition active?}
B -- Yes --> C[Ignore duplicate navigation]
B -- No --> D[Start wipe transition]
D --> E{Client router commits?}
E -- Yes --> F[Reveal destination route]
E -- No after 8000ms --> G[Native location.assign]
D --> H{Router navigation throws?}
H -- Yes --> G
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/components/appearance/p3r-effects.tsx" line_range="114" />
<code_context>
const request = (event: Event) => {
if (motion.matches || document.hidden || useRainConnection.getState().connection) return
event.preventDefault()
+ if (active) return
start((event as CustomEvent<P3RWipeRequest>).detail)
}
</code_context>
<issue_to_address>
**Second destination never opens**
When a user activates another P3RLink while a wipe is active, `request` calls `preventDefault()` before checking `active`, then returns without starting the request; `P3RLink` treats it as accepted and cancels the link navigation, so the second destination never opens.
Check `active` before preventing the request, and let the link navigation proceed when a wipe is already active.
</issue_to_address>
### Comment 2
<location path="src/components/appearance/p3r-effects.tsx" line_range="49" />
<code_context>
- navigate?.()
- }
- const start = ({ x, y, navigate }: P3RWipeRequest) => {
+ const skip = () => clear()
+ const start = ({ x, y, navigate, fallback }: P3RWipeRequest) => {
clear()
</code_context>
<issue_to_address>
**Stalled route loses fallback**
When a client route has started and the user skips the wipe before it commits, `skip()` calls `clear()`, which cancels the route fallback timer; if the route does not commit, no native navigation occurs and the user remains on the old page.
Preserve or rearm the route fallback timer when skipping an in-flight navigation.
</issue_to_address>
### Comment 3
<location path="src/components/appearance/p3r-effects.tsx" line_range="90-94" />
<code_context>
- // A failed route must never leave an undismissable visual layer.
- timeout = window.setTimeout(clear, 12000)
+ // A stalled client route must still reach its destination.
+ timeout = window.setTimeout(() => {
+ committed.current = null
+ if (!fallback) { clear(); return }
+ try { fallback() } catch { clear() }
+ }, ROUTE_FALLBACK_MS)
}
if (ready && now - started >= 440) {
</code_context>
<issue_to_address>
**Committed route reloads**
When the pathname commits but animation frames remain stalled until the fallback timer fires, the timeout calls `fallback()` without checking `ready`, so `location.assign()` reloads the destination despite the completed client transition and can discard client-side state.
Skip the fallback when the route has already committed, and clear the timer on commit.
</issue_to_address>
### Comment 4
<location path="src/components/appearance/p3r-link.tsx" line_range="41" />
<code_context>
return true
+ }, fallback: () => {
+ const destination = `${url.pathname}${url.search}${url.hash}`
+ location.assign(destination)
} })
if (accepted) event.preventDefault()
</code_context>
<issue_to_address>
**Replace adds history entry**
When a `replace` navigation reaches the native fallback after the client route fails to commit, the fallback calls `location.assign()` instead of replacing the current entry, so the destination is added to browser history and Back returns to the page that was meant to be replaced.
Use `location.replace()` for replace navigations and `location.assign()` for other navigations.
</issue_to_address>
### Comment 5
<location path="src/lib/server/public-content.ts" line_range="51" />
<code_context>
export const getArticle = cache(async (id: string) => {
try {
- const result = await publicEnvelope<Entry>(`knowledge/${id}`, true)
+ const result = await publicEnvelope<Entry>(`knowledge/${id}`)
return result.data
</code_context>
<issue_to_address>
**Like counts stay outdated**
When an article or moment receives a like while its detail response is cached, `getArticle` and `getMoment` cache detail payloads, including like counts, for up to 300 seconds, while like requests do not invalidate `public-blog-content`; other visitors therefore see outdated counts.
Invalidate `public-blog-content` after successful article and moment like requests.
Also at `src/lib/server/public-content.ts:48`, `src/lib/server/public-content.ts:86`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 5 findings to address first, and the change alters client navigation behavior and changes article and moment detail fetches from uncached requests to revalidated caching. If incorrect, navigation may fall back to a full reload after a delay, and stale detail content could be served for the cache window; the latter outlives a revert but is bounded and repairable through revalidation or cache invalidation.
Blocking findings: src/components/appearance/p3r-effects.tsx:114, src/components/appearance/p3r-effects.tsx:49, src/components/appearance/p3r-effects.tsx:94, src/components/appearance/p3r-link.tsx:41, src/lib/server/public-content.ts:51
| const request = (event: Event) => { | ||
| if (motion.matches || document.hidden || useRainConnection.getState().connection) return | ||
| event.preventDefault() | ||
| if (active) return |
There was a problem hiding this comment.
🟡 Medium · Second destination never opens
When a user activates another P3RLink while a wipe is active, request calls preventDefault() before checking active, then returns without starting the request; P3RLink treats it as accepted and cancels the link navigation, so the second destination never opens.
Check active before preventing the request, and let the link navigation proceed when a wipe is already active.
Prompt for AI agents
In `src/components/appearance/p3r-effects.tsx` at line 114:
**Second destination never opens**
When a user activates another P3RLink while a wipe is active, `request` calls `preventDefault()` before checking `active`, then returns without starting the request; `P3RLink` treats it as accepted and cancels the link navigation, so the second destination never opens.
Check `active` before preventing the request, and let the link navigation proceed when a wipe is already active.| navigate?.() | ||
| } | ||
| const start = ({ x, y, navigate }: P3RWipeRequest) => { | ||
| const skip = () => clear() |
There was a problem hiding this comment.
🟡 Medium · Stalled route loses fallback
When a client route has started and the user skips the wipe before it commits, skip() calls clear(), which cancels the route fallback timer; if the route does not commit, no native navigation occurs and the user remains on the old page.
Preserve or rearm the route fallback timer when skipping an in-flight navigation.
Prompt for AI agents
In `src/components/appearance/p3r-effects.tsx` at line 49:
**Stalled route loses fallback**
When a client route has started and the user skips the wipe before it commits, `skip()` calls `clear()`, which cancels the route fallback timer; if the route does not commit, no native navigation occurs and the user remains on the old page.
Preserve or rearm the route fallback timer when skipping an in-flight navigation.
Comment on lines
+90
to
+94
| timeout = window.setTimeout(() => { | ||
| committed.current = null | ||
| if (!fallback) { clear(); return } | ||
| try { fallback() } catch { clear() } | ||
| }, ROUTE_FALLBACK_MS) |
There was a problem hiding this comment.
🟡 Medium · Committed route reloads
When the pathname commits but animation frames remain stalled until the fallback timer fires, the timeout calls fallback() without checking ready, so location.assign() reloads the destination despite the completed client transition and can discard client-side state.
Skip the fallback when the route has already committed, and clear the timer on commit.
Prompt for AI agents
In `src/components/appearance/p3r-effects.tsx` at lines 90-94:
**Committed route reloads**
When the pathname commits but animation frames remain stalled until the fallback timer fires, the timeout calls `fallback()` without checking `ready`, so `location.assign()` reloads the destination despite the completed client transition and can discard client-side state.
Skip the fallback when the route has already committed, and clear the timer on commit.| return true | ||
| }, fallback: () => { | ||
| const destination = `${url.pathname}${url.search}${url.hash}` | ||
| location.assign(destination) |
There was a problem hiding this comment.
🟡 Medium · Replace adds history entry
When a replace navigation reaches the native fallback after the client route fails to commit, the fallback calls location.assign() instead of replacing the current entry, so the destination is added to browser history and Back returns to the page that was meant to be replaced.
Use location.replace() for replace navigations and location.assign() for other navigations.
Prompt for AI agents
In `src/components/appearance/p3r-link.tsx` at line 41:
**Replace adds history entry**
When a `replace` navigation reaches the native fallback after the client route fails to commit, the fallback calls `location.assign()` instead of replacing the current entry, so the destination is added to browser history and Back returns to the page that was meant to be replaced.
Use `location.replace()` for replace navigations and `location.assign()` for other navigations.|
|
||
| export const getArticle = cache(async (id: string) => { | ||
| try { | ||
| const result = await publicEnvelope<Entry>(`knowledge/${id}`, true) |
There was a problem hiding this comment.
🟡 Medium · Like counts stay outdated
When an article or moment receives a like while its detail response is cached, getArticle and getMoment cache detail payloads, including like counts, for up to 300 seconds, while like requests do not invalidate public-blog-content; other visitors therefore see outdated counts.
Invalidate public-blog-content after successful article and moment like requests.
Also at src/lib/server/public-content.ts:48, src/lib/server/public-content.ts:86.
Prompt for AI agents
In `src/lib/server/public-content.ts` at line 51:
**Like counts stay outdated**
When an article or moment receives a like while its detail response is cached, `getArticle` and `getMoment` cache detail payloads, including like counts, for up to 300 seconds, while like requests do not invalidate `public-blog-content`; other visitors therefore see outdated counts.
Invalidate `public-blog-content` after successful article and moment like requests.
Also at `src/lib/server/public-content.ts:48`, `src/lib/server/public-content.ts:86`.This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Make blog navigation transitions reliable with route loading feedback and robust client-navigation fallbacks.
New Features:
Bug Fixes:
Enhancements: