-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: add view transition events #5861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds ViewTransition types and a LocationChangeInfo type to router core; threads a single computed location-change context through navigation and load flows; updates startViewTransition to accept LocationChangeInfo and emit four view-transition lifecycle events (start, ready, update-callback-done, finish) carrying both transition and location-change data; documents a new Changes
Sequence Diagram(s)sequenceDiagram
participant Router as Router
participant DOM as Document API
participant Listener as Event Listener
Router->>Router: startViewTransition(locationChangeInfo, fn)
Router->>DOM: document.startViewTransition(fn)
DOM-->>Router: transition
rect `#E8F0FF`
Router->>Listener: emit onViewTransitionStart (transition + locationChangeInfo)
end
rect `#FFF4E6`
DOM->>Router: transition.ready
Router->>Listener: emit onViewTransitionReady (transition + locationChangeInfo)
end
rect `#F6FFF2`
DOM->>Router: transition.updateCallbackDone
Router->>Listener: emit onViewTransitionUpdateCallbackDone (transition + locationChangeInfo)
end
rect `#F0FFF7`
DOM->>Router: transition.finished
Router->>Listener: emit onViewTransitionFinish (transition + locationChangeInfo)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/router-core/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (6)📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
📚 Learning: 2025-09-23T17:36:12.598ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
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 |
|
View your CI Pipeline Execution ↗ for commit 6b9283b
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/router.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/src/router.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/router.ts
🧠 Learnings (2)
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
packages/router-core/src/router.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
packages/router-core/src/router.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
c6c270d to
813e9a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/router-core/src/router.ts (1)
2312-2338: Handle failed view-transition promises to avoid unhandled rejections.The
.then(...)calls ontransition.ready,transition.updateCallbackDone, andtransition.finishedlack rejection handlers. These promises reject in legitimate scenarios (e.g., duplicateview-transition-namevalues or rejected update callbacks), causing "Unhandled promise rejection" console errors even though the router gracefully falls back to non-animated updates.Apply this diff to add rejection handlers:
- transition.ready.then(() => { + transition.ready.then( + () => { this.emit({ type: 'onViewTransitionReady', transition, ...locationChangeInfo, }) - }) - transition.updateCallbackDone.then(() => { + }, + () => {}, + ) + transition.updateCallbackDone.then( + () => { this.emit({ type: 'onViewTransitionUpdateCallbackDone', transition, ...locationChangeInfo, }) - }) - transition.finished.then(() => { + }, + () => {}, + ) + transition.finished.then( + () => { this.emit({ type: 'onViewTransitionFinish', transition, ...locationChangeInfo, }) - }) + }, + () => {}, + )
🧹 Nitpick comments (1)
packages/router-core/src/router.ts (1)
2286-2296: Consider reusing the locationChangeInfo parameter.Lines 2292-2295 recompute
getLocationChangeInfo, but this function already received a pre-computedlocationChangeInfoparameter. Reusing the parameter eliminates redundant computation.Apply this diff:
if ( typeof shouldViewTransition === 'object' && this.isViewTransitionTypesSupported ) { - const next = this.latestLocation - const prevLocation = this.state.resolvedLocation - const resolvedViewTransitionTypes = typeof shouldViewTransition.types === 'function' - ? shouldViewTransition.types( - getLocationChangeInfo({ - resolvedLocation: prevLocation, - location: next, - }), - ) + ? shouldViewTransition.types(locationChangeInfo) : shouldViewTransition.types
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/router/framework/react/api/router/RouterEventsType.md(4 hunks)packages/router-core/src/router.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use internal docs links relative to the docs/ folder (e.g., ./guide/data-loading)
Files:
docs/router/framework/react/api/router/RouterEventsType.md
docs/{router,start}/**
📄 CodeRabbit inference engine (AGENTS.md)
Place router docs under docs/router/ and start framework docs under docs/start/
Files:
docs/router/framework/react/api/router/RouterEventsType.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/src/router.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/router.ts
🧠 Learnings (3)
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Applied to files:
docs/router/framework/react/api/router/RouterEventsType.md
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
packages/router-core/src/router.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
packages/router-core/src/router.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (9)
packages/router-core/src/router.ts (5)
547-550: LGTM! Clean type definition for view transition events.The
ViewTransitionEventInfotype properly encapsulates theViewTransitioninstance, and the@ts-ignorecomment is appropriate given thatViewTransitionsupport requires TypeScript 5.6+.
571-587: LGTM! Well-structured view transition events.The four new events properly mirror the View Transition API lifecycle and correctly combine
ViewTransitionEventInfoandNavigationEventInfo. The type structure ensures consumers receive both the transition instance and location-change context.
851-851: Good type alias for reusability.The
LocationChangeInfotype alias provides a clean, reusable type for location-change data.
2113-2128: Excellent optimization to compute location-change info once.Computing
locationChangeInfoupfront and reusing it across multiple event emissions eliminates redundant calculations and ensures consistency.
2260-2263: Function signature properly updated.The
startViewTransitionmethod now correctly acceptslocationChangeInfoas a parameter, enabling the emission of view transition events with location-change context.docs/router/framework/react/api/router/RouterEventsType.md (4)
16-61: Documentation accurately reflects hashChanged additions.The
hashChangedproperty is correctly documented for all navigation lifecycle events, matching the code implementation.
62-97: Comprehensive documentation of view transition events.All four view transition lifecycle events are clearly documented with complete property lists that accurately match the TypeScript definitions.
107-107: Type union properly updated.The event type union now includes all four new view transition events.
131-142: Clear documentation of new properties.The
hashChangedandtransitionproperties are well-documented with appropriate type information and helpful context. The MDN link forViewTransitionis a valuable reference.
|
@ha1fstack , I've made two e2e suites (one for react-router, one for solid-router) in here, that are ready to add tests after rebasing this PR. If you add some cases/routes in the app.spec.ts to check the events fire as expected, then we can move this forward. |
Added the following router events so we could access view transition events and ViewTransition instance.
Summary by CodeRabbit
New Features
Documentation