Skip to content

Commit 3afbb02

Browse files
TkDodoandrewshie-sentry
authored andcommitted
fix(ui): DocumentTitleManager for predictable sentry document titles (#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
1 parent e248920 commit 3afbb02

File tree

9 files changed

+253
-166
lines changed

9 files changed

+253
-166
lines changed

static/app/bootstrap/processInitQueue.tsx

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import throttle from 'lodash/throttle';
55

66
import {exportedGlobals} from 'sentry/bootstrap/exportGlobals';
77
import {CommandPaletteProvider} from 'sentry/components/commandPalette/context';
8+
import {DocumentTitleManager} from 'sentry/components/sentryDocumentTitle/documentTitleManager';
89
import {ThemeAndStyleProvider} from 'sentry/components/themeAndStyleProvider';
910
import {ScrapsProviders} from 'sentry/scrapsProviders';
1011
import type {OnSentryInitConfiguration} from 'sentry/types/system';
@@ -111,17 +112,19 @@ async function processItem(initConfig: OnSentryInitConfiguration) {
111112
* and so we dont know which theme to pick.
112113
*/
113114
<QueryClientProvider client={queryClient}>
114-
<ThemeAndStyleProvider>
115-
<CommandPaletteProvider>
116-
<SimpleRouter
117-
element={
118-
<ScrapsProviders>
119-
<Component {...props} />
120-
</ScrapsProviders>
121-
}
122-
/>
123-
</CommandPaletteProvider>
124-
</ThemeAndStyleProvider>
115+
<DocumentTitleManager>
116+
<ThemeAndStyleProvider>
117+
<CommandPaletteProvider>
118+
<SimpleRouter
119+
element={
120+
<ScrapsProviders>
121+
<Component {...props} />
122+
</ScrapsProviders>
123+
}
124+
/>
125+
</CommandPaletteProvider>
126+
</ThemeAndStyleProvider>
127+
</DocumentTitleManager>
125128
</QueryClientProvider>
126129
),
127130
initConfig.container,

static/app/components/sentryDocumentTitle.spec.tsx

Lines changed: 0 additions & 48 deletions
This file was deleted.

static/app/components/sentryDocumentTitle.tsx

Lines changed: 0 additions & 86 deletions
This file was deleted.
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import React, {createContext, useContext, useEffect, useMemo, useState} from 'react';
2+
3+
const DEFAULT_PAGE_TITLE = 'Sentry';
4+
const SEPARATOR = ' — ';
5+
6+
interface TitleEntry {
7+
id: string;
8+
noSuffix: boolean;
9+
order: number;
10+
text: string;
11+
}
12+
13+
interface DocumentTitleManager {
14+
register: (id: string, text: string, order: number, noSuffix: boolean) => void;
15+
unregister: (id: string) => void;
16+
}
17+
18+
const DocumentTitleContext = createContext<DocumentTitleManager>({
19+
register: () => {},
20+
unregister: () => {},
21+
});
22+
23+
export const useDocumentTitleManager = () => useContext(DocumentTitleContext);
24+
25+
export function DocumentTitleManager({children}: React.PropsWithChildren) {
26+
const [entries, setEntries] = useState<TitleEntry[]>([]);
27+
28+
const [manager] = useState<DocumentTitleManager>(() => ({
29+
register: (id, text, order, noSuffix) => {
30+
setEntries(prev => {
31+
// update for same id
32+
if (prev.some(e => e.id === id)) {
33+
return prev.map(e => (e.id === id ? {...e, text, noSuffix} : e));
34+
}
35+
return [...prev, {id, text, noSuffix, order}];
36+
});
37+
},
38+
unregister: id => {
39+
setEntries(prev => prev.filter(e => e.id !== id));
40+
},
41+
}));
42+
43+
const fullTitle = useMemo(() => {
44+
const entry = entries
45+
.filter(e => e.text.trim() !== '')
46+
.sort((a, b) => b.order - a.order)
47+
.at(0);
48+
49+
const parts = entry ? [entry.text] : [];
50+
51+
if (!entry?.noSuffix) {
52+
parts.push(DEFAULT_PAGE_TITLE);
53+
}
54+
return [...new Set([...parts])].join(SEPARATOR);
55+
}, [entries]);
56+
57+
// write to the DOM title
58+
useEffect(() => {
59+
if (fullTitle.length > 0) {
60+
document.title = fullTitle;
61+
}
62+
}, [fullTitle]);
63+
64+
return (
65+
<DocumentTitleContext.Provider value={manager}>
66+
{children}
67+
</DocumentTitleContext.Provider>
68+
);
69+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import {render} from 'sentry-test/reactTestingLibrary';
2+
3+
import {DocumentTitleManager} from './documentTitleManager';
4+
import SentryDocumentTitle from '.';
5+
6+
describe('SentryDocumentTitle', () => {
7+
it('sets the document title', () => {
8+
render(
9+
<DocumentTitleManager>
10+
<SentryDocumentTitle title="This is a test" />
11+
</DocumentTitleManager>
12+
);
13+
expect(document.title).toBe('This is a test — Sentry');
14+
});
15+
16+
it('adds a organization slug', () => {
17+
render(
18+
<DocumentTitleManager>
19+
<SentryDocumentTitle orgSlug="org" title="This is a test" />
20+
</DocumentTitleManager>
21+
);
22+
expect(document.title).toBe('This is a test — org — Sentry');
23+
});
24+
25+
it('adds a project slug', () => {
26+
render(
27+
<DocumentTitleManager>
28+
<SentryDocumentTitle projectSlug="project" title="This is a test" />
29+
</DocumentTitleManager>
30+
);
31+
expect(document.title).toBe('This is a test — project — Sentry');
32+
});
33+
34+
it('adds a organization and project slug', () => {
35+
render(
36+
<DocumentTitleManager>
37+
<SentryDocumentTitle orgSlug="org" projectSlug="project" title="This is a test" />
38+
</DocumentTitleManager>
39+
);
40+
expect(document.title).toBe('This is a test — org — project — Sentry');
41+
});
42+
43+
it('sets the title without suffix', () => {
44+
render(
45+
<DocumentTitleManager>
46+
<SentryDocumentTitle title="This is a test" noSuffix />
47+
</DocumentTitleManager>
48+
);
49+
expect(document.title).toBe('This is a test');
50+
});
51+
52+
it('reverts to the parent title', () => {
53+
const {rerender} = render(
54+
<DocumentTitleManager>
55+
<SentryDocumentTitle title="This is a test">
56+
<SentryDocumentTitle title="child title">Content</SentryDocumentTitle>
57+
</SentryDocumentTitle>
58+
</DocumentTitleManager>
59+
);
60+
61+
expect(document.title).toBe('child title — Sentry');
62+
63+
rerender(
64+
<DocumentTitleManager>
65+
<SentryDocumentTitle title="This is a test">new Content</SentryDocumentTitle>
66+
</DocumentTitleManager>
67+
);
68+
69+
expect(document.title).toBe('This is a test — Sentry');
70+
});
71+
});
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import {useEffect, useId, useMemo, useState} from 'react';
2+
3+
import {useDocumentTitleManager} from './documentTitleManager';
4+
5+
type Props = {
6+
children?: React.ReactNode;
7+
/**
8+
* Should the ` - Sentry` suffix be excluded?
9+
*/
10+
noSuffix?: boolean;
11+
/**
12+
* The organization slug to show in the title
13+
*/
14+
orgSlug?: string;
15+
/**
16+
* The project slug to show in the title.
17+
*/
18+
projectSlug?: string;
19+
20+
/**
21+
* This string will be shown at the very front of the title
22+
*/
23+
title?: string;
24+
};
25+
26+
function SentryDocumentTitle({
27+
title = '',
28+
orgSlug,
29+
projectSlug,
30+
noSuffix,
31+
children,
32+
}: Props) {
33+
const titleManager = useDocumentTitleManager();
34+
const id = useId();
35+
// compute order once on mount because effects run bottom-up
36+
const [order] = useState(() => performance.now());
37+
38+
const pageTitle = useMemo(() => {
39+
if (orgSlug && projectSlug) {
40+
return `${title}${orgSlug}${projectSlug}`;
41+
}
42+
43+
if (orgSlug) {
44+
return `${title}${orgSlug}`;
45+
}
46+
47+
if (projectSlug) {
48+
return `${title}${projectSlug}`;
49+
}
50+
51+
return title;
52+
}, [orgSlug, projectSlug, title]);
53+
54+
// create or update title entry
55+
useEffect(() => {
56+
titleManager.register(id, pageTitle, order, !!noSuffix);
57+
}, [titleManager, id, pageTitle, order, noSuffix]);
58+
59+
// cleanup on unmount
60+
useEffect(() => {
61+
return () => {
62+
titleManager.unregister(id);
63+
};
64+
}, [titleManager, id]);
65+
66+
return children;
67+
}
68+
69+
export default SentryDocumentTitle;

0 commit comments

Comments
 (0)