-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(ui): DocumentTitleManager for predictable sentry document titles #103150
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
Merged
+253
−166
Conversation
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
TkDodo
commented
Nov 11, 2025
evanpurkhiser
approved these changes
Nov 11, 2025
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues: |
Jesse-Box
pushed a commit
that referenced
this pull request
Nov 12, 2025
…103150) Right now, every instance of `<SentryDocumentTitle>` computes a title, applies it to the `document.title` and stores its value in context, so that the next `<SentryDocumentTitle>` can pick that up and “revert” to that state. However, there are situations where this doesn’t work so well because the unmount effect that attempts to clean things up might run after the next component has rendered, leading to wrong titles in some situations where the title just resets to `Sentry`: https://github.com/user-attachments/assets/016c4e36-16d0-4a86-86fc-b27f7800db22 The reason why it mostly works is because we have additional re-renders thanks to `useLocation()` at the top of most pages, which will restore the correct title. But this is brittle and as shown, doesn’t work in all cases. This PR re-writes takes a different approach to applying document titles. Instead of writing to the title on each level in render (which can also lead to flickering), we just register with a global `DocumentTitleManager` in an effect. To make sure we have the right order, we compute a timestamp once during render (in `useState`) when the component mounts and pass that along the text we want to the manager. The `DocumentTitleManager` itself then just knows all the registered titles, and displays the one we want (the last one, and optionally add the default title to it. This also gives us the possibility to do “compound titles” like in breadcrumbs if we want to). To get the last item, it uses the insertion order that was calculated during render, so the fact that `useEffect` runs bottom-up doesn’t matter. Note that we can’t just `reverse()` the array because data fetching might mean some effects run sooner than others. For example, let’s say we add two titles, then fetch, then add two more. rendering goes like this: ``` 1 -- 2 -- ...fetching... -- 3 -- 4 ``` but effects will run like this (bottom up, for reach committed group individually): ``` 2 -- 1 -- ...fetching... -- 4 -- 3 ``` yielding a result order of `2,1,3,4`, which isn’t reversible. The timestamp computed on mount resolves this issue, because we have something stable to sort by. For cleanup, when a component unmounts, it just removes itself (by id) from the manager, which will automatically lead to the “previously registered” title to show up. This can go over an arbitrary number of layers, while the previous solution was limited to one parent. --- after: https://github.com/user-attachments/assets/a278fc4a-7753-4ce3-a8ca-a4b9a2a9ae58
andrewshie-sentry
pushed a commit
that referenced
this pull request
Nov 13, 2025
…103150) Right now, every instance of `<SentryDocumentTitle>` computes a title, applies it to the `document.title` and stores its value in context, so that the next `<SentryDocumentTitle>` can pick that up and “revert” to that state. However, there are situations where this doesn’t work so well because the unmount effect that attempts to clean things up might run after the next component has rendered, leading to wrong titles in some situations where the title just resets to `Sentry`: https://github.com/user-attachments/assets/016c4e36-16d0-4a86-86fc-b27f7800db22 The reason why it mostly works is because we have additional re-renders thanks to `useLocation()` at the top of most pages, which will restore the correct title. But this is brittle and as shown, doesn’t work in all cases. This PR re-writes takes a different approach to applying document titles. Instead of writing to the title on each level in render (which can also lead to flickering), we just register with a global `DocumentTitleManager` in an effect. To make sure we have the right order, we compute a timestamp once during render (in `useState`) when the component mounts and pass that along the text we want to the manager. The `DocumentTitleManager` itself then just knows all the registered titles, and displays the one we want (the last one, and optionally add the default title to it. This also gives us the possibility to do “compound titles” like in breadcrumbs if we want to). To get the last item, it uses the insertion order that was calculated during render, so the fact that `useEffect` runs bottom-up doesn’t matter. Note that we can’t just `reverse()` the array because data fetching might mean some effects run sooner than others. For example, let’s say we add two titles, then fetch, then add two more. rendering goes like this: ``` 1 -- 2 -- ...fetching... -- 3 -- 4 ``` but effects will run like this (bottom up, for reach committed group individually): ``` 2 -- 1 -- ...fetching... -- 4 -- 3 ``` yielding a result order of `2,1,3,4`, which isn’t reversible. The timestamp computed on mount resolves this issue, because we have something stable to sort by. For cleanup, when a component unmounts, it just removes itself (by id) from the manager, which will automatically lead to the “previously registered” title to show up. This can go over an arbitrary number of layers, while the previous solution was limited to one parent. --- after: https://github.com/user-attachments/assets/a278fc4a-7753-4ce3-a8ca-a4b9a2a9ae58
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.
Right now, every instance of
<SentryDocumentTitle>computes a title, applies it to thedocument.titleand stores its value in context, so that the next<SentryDocumentTitle>can pick that up and “revert” to that state.However, there are situations where this doesn’t work so well because the unmount effect that attempts to clean things up might run after the next component has rendered, leading to wrong titles in some situations where the title just resets to
Sentry:Screen.Recording.2025-11-11.at.11.17.57.mov
The reason why it mostly works is because we have additional re-renders thanks to
useLocation()at the top of most pages, which will restore the correct title. But this is brittle and as shown, doesn’t work in all cases.This PR re-writes takes a different approach to applying document titles. Instead of writing to the title on each level in render (which can also lead to flickering), we just register with a global
DocumentTitleManagerin an effect. To make sure we have the right order, we compute a timestamp once during render (inuseState) when the component mounts and pass that along the text we want to the manager.The
DocumentTitleManageritself then just knows all the registered titles, and displays the one we want (the last one, and optionally add the default title to it. This also gives us the possibility to do “compound titles” like in breadcrumbs if we want to). To get the last item, it uses the insertion order that was calculated during render, so the fact thatuseEffectruns bottom-up doesn’t matter.Note that we can’t just
reverse()the array because data fetching might mean some effects run sooner than others. For example, let’s say we add two titles, then fetch, then add two more. rendering goes like this:but effects will run like this (bottom up, for reach committed group individually):
yielding a result order of
2,1,3,4, which isn’t reversible. The timestamp computed on mount resolves this issue, because we have something stable to sort by.For cleanup, when a component unmounts, it just removes itself (by id) from the manager, which will automatically lead to the “previously registered” title to show up. This can go over an arbitrary number of layers, while the previous solution was limited to one parent.
after:
Screen.Recording.2025-11-11.at.15.17.48.mov