Skip to content

Commit 9a8aa7b

Browse files
committed
🐛 Fix memory leak and hardcoded timeout in client-side navigation
Code review fixes: - Clear timeout when storyRendered event fires (prevents timer leak) - Use configurable timeout parameter instead of hardcoded 5000ms - Add debug logging before falling back to full page navigation
1 parent 6132231 commit 9a8aa7b

1 file changed

Lines changed: 36 additions & 25 deletions

File tree

clients/storybook/src/navigation.js

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,12 @@ export async function navigateToStory(tab, storyId, baseUrl, options = {}) {
4949
try {
5050
await clientSideNavigation(tab, storyId, timeout);
5151
entry.currentStoryId = storyId;
52-
} catch {
53-
// Fallback to full navigation if client-side fails
52+
} catch (error) {
53+
// Log and fallback to full navigation if client-side fails
54+
console.debug?.(
55+
`Client-side navigation failed for ${storyId}, falling back to full page load:`,
56+
error.message
57+
);
5458
await fullPageNavigation(tab, storyId, baseUrl, timeout);
5559
entry.currentStoryId = storyId;
5660
}
@@ -94,33 +98,40 @@ async function fullPageNavigation(tab, storyId, baseUrl, timeout) {
9498
* @param {string} storyId - Story ID
9599
* @param {number} timeout - Timeout in ms
96100
*/
97-
async function clientSideNavigation(tab, storyId, _timeout) {
101+
async function clientSideNavigation(tab, storyId, timeout) {
98102
// Navigate using Storybook's preview API and wait for story to render
99-
await tab.evaluate(id => {
100-
return new Promise((resolve, reject) => {
101-
let preview = window.__STORYBOOK_PREVIEW__;
102-
if (!preview?.channel) {
103-
reject(new Error('Storybook preview API not available'));
104-
return;
105-
}
103+
await tab.evaluate(
104+
(id, timeoutMs) => {
105+
return new Promise((resolve, reject) => {
106+
let preview = window.__STORYBOOK_PREVIEW__;
107+
if (!preview?.channel) {
108+
reject(new Error('Storybook preview API not available'));
109+
return;
110+
}
106111

107-
// Listen for story render completion
108-
let handleRendered = () => {
109-
preview.channel.off('storyRendered', handleRendered);
110-
resolve();
111-
};
112-
preview.channel.on('storyRendered', handleRendered);
112+
let timeoutId;
113113

114-
// Navigate to the story
115-
preview.channel.emit('setCurrentStory', { storyId: id });
114+
// Listen for story render completion
115+
let handleRendered = () => {
116+
clearTimeout(timeoutId);
117+
preview.channel.off('storyRendered', handleRendered);
118+
resolve();
119+
};
120+
preview.channel.on('storyRendered', handleRendered);
116121

117-
// Timeout fallback
118-
setTimeout(() => {
119-
preview.channel.off('storyRendered', handleRendered);
120-
resolve(); // Resolve anyway - story might have rendered
121-
}, 5000);
122-
});
123-
}, storyId);
122+
// Navigate to the story
123+
preview.channel.emit('setCurrentStory', { storyId: id });
124+
125+
// Timeout fallback - use configured timeout
126+
timeoutId = setTimeout(() => {
127+
preview.channel.off('storyRendered', handleRendered);
128+
resolve(); // Resolve anyway - story might have rendered
129+
}, timeoutMs);
130+
});
131+
},
132+
storyId,
133+
timeout
134+
);
124135
}
125136

126137
/**

0 commit comments

Comments
 (0)