fix(nextjs): Keep router.back/forward navigation type under span streaming - #24074
Open
chargome wants to merge 3 commits into
Open
fix(nextjs): Keep router.back/forward navigation type under span streaming#24074chargome wants to merge 3 commits into
chargome wants to merge 3 commits into
Conversation
…aming In router-patch mode, router.back() and router.forward() started a navigation span with a placeholder name that the popstate listener renamed later. With span streaming, ignoreSpans is applied when a span starts, so the placeholder span was non-recording from the start and popstate fell through to creating a second span tagged browser.popstate. Instead of the placeholder span, remember the router method and its timestamp, and start the navigation span from the popstate event with that navigation type and start time. This keeps the span starting at the router call, works for both trace lifecycles, and no longer records an ignored-span client outcome per traversal. A pending traversal expires after one second so a back()/forward() with no matching history entry cannot be attributed to a later unrelated popstate. The placeholder name and its ignoreSpans entry are removed since nothing produces such spans anymore. Fixes #23909 Refs JS-3527 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
size-limit report 📦
|
A timestamp comparison misattributed the popstate when the main thread was blocked between router.back() and the event, since the delay counts against wall-clock age. A timer is delayed by the same block, which is how the previous placeholder span's idle timeout behaved. Also cover the branch that starts a fresh span when back() is called while a push() span is still open. Refs JS-3527 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
|
bugbot run |
Spans started from popstate built their URL from the pathname alone, so router.back()/forward() lost the query string in url.full that the previous update path preserved from location.href. Pass the full location instead, which also aligns plain browser.popstate spans. Drive the traversal expiry test with fake timers instead of a real sleep. Refs JS-3527 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 25cf59a. Configure here.
chargome
marked this pull request as ready for review
September 4, 2026 12:37
chargome
requested review from
RulaKhaled,
logaretm,
mydea and
s1gr1d
and removed request for
a team and
mydea
September 4, 2026 12:37
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.
Under span streaming,
ignoreSpansis applied at span start. The placeholder span thatrouter.back()/router.forward()started in router-patch mode was therefore non-recording, andpopstatecreated a second span taggedbrowser.popstateinstead of renaming it.back()/forward()now record the router method and timestamp, and thepopstatelistener starts the navigation span with thatnavigation.typeandstartTime. The placeholder name and itsignoreSpansentry are removed, and the two E2E tests skipped in #23905 are re-enabled.Fixes #23909