fix: stop the render loop that crashes the mobile app switcher (#17)

`mapWindow` flagged the state as changed whenever the target window id
matched, even when the updater handed the very same window object back.
`SET_TITLE` is guarded against a no-op title, but that guard was defeated:
the reducer still returned a fresh state object, so the context value
changed, every consumer re-rendered, and each app's `setTitle` effect —
keyed on a `setTitle` callback that the mobile shell re-created on every
render — dispatched `SET_TITLE` again. That loop ran continuously while an
app was open on mobile; opening the app-switcher Sheet on top of it made
Radix re-attach its composed refs on every one of those renders until
React bailed out with "Maximum update depth exceeded" and the
ErrorBoundary took over.

Only count a window as changed when the updater actually returned a
different object, and memoise the mobile shell's `setTitle`/`setParams`
per window, matching what `WindowFrame` already does.

Fixes #14


Claude-Session: https://claude.ai/code/session_01Trku191Ww2a2YmDWQF3fTS

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
mroxso
2026-09-06 12:09:11 +02:00
committed by GitHub
parent cde5be0874
commit dc1701afd9
3 changed files with 80 additions and 5 deletions

View File

@@ -1,4 +1,4 @@
import { Suspense, useMemo, useState } from 'react';
import { Suspense, useCallback, useMemo, useState } from 'react';
import { ChevronLeft, LayoutGrid, Zap } from 'lucide-react';
import { Skeleton } from '@/components/ui/skeleton';
import { ErrorBoundary } from '@/components/ErrorBoundary';
@@ -14,6 +14,7 @@ import {
import { useWindowManager } from '@/os/useWindowManager';
import { desktopApps, getApp } from '@/os/registry';
import { cn } from '@/lib/utils';
import type { AppParams } from '@/os/types';
/**
* On a phone the window metaphor only gets in the way, so the same apps and
@@ -30,6 +31,23 @@ export function MobileAppShell() {
[windows, focusedId],
);
const activeApp = active ? getApp(active.appId) : undefined;
const activeId = active?.id;
// Stable per-window callbacks: apps set their title from an effect keyed on
// `setTitle`, so a fresh function on every shell render would re-run it.
const setTitle = useCallback(
(title: string) => {
if (activeId) setWindowTitle(activeId, title);
},
[activeId, setWindowTitle],
);
const setParams = useCallback(
(params: AppParams) => {
if (activeId) setWindowParams(activeId, params);
},
[activeId, setWindowParams],
);
return (
<div className="flex h-full flex-col bg-background">
@@ -108,8 +126,8 @@ export function MobileAppShell() {
<activeApp.component
windowId={active.id}
params={active.params}
setTitle={(title) => setWindowTitle(active.id, title)}
setParams={(params) => setWindowParams(active.id, params)}
setTitle={setTitle}
setParams={setParams}
/>
</Suspense>
</ErrorBoundary>

View File

@@ -0,0 +1,53 @@
import { lazy } from 'react';
import { describe, expect, it } from 'vitest';
import { Info } from 'lucide-react';
import { initialState, windowReducer } from './windowReducer';
import type { AppDefinition } from './types';
const viewport = { width: 1280, height: 800 };
const app: AppDefinition = {
id: 'about',
title: 'About',
description: 'What this is and how it works',
icon: Info,
category: 'system',
component: lazy(async () => ({ default: () => null })),
defaultSize: { width: 400, height: 400 },
minSize: { width: 200, height: 200 },
};
function openOne() {
const state = windowReducer(initialState, { type: 'OPEN_APP', app, viewport });
return { state, id: state.windows[0].id };
}
describe('windowReducer', () => {
it('opens an app and focuses it', () => {
const { state, id } = openOne();
expect(state.windows).toHaveLength(1);
expect(state.focusedId).toBe(id);
});
// Apps set their window title from an effect that re-runs whenever the
// callback identity changes. Handing back a fresh state for a title that did
// not change re-renders every context consumer, which re-runs the effect,
// which dispatches again — an endless render loop.
it('returns the same state when a no-op title is set', () => {
const { state, id } = openOne();
const next = windowReducer(state, { type: 'SET_TITLE', id, title: 'About' });
expect(next).toBe(state);
});
it('still applies a title that differs', () => {
const { state, id } = openOne();
const next = windowReducer(state, { type: 'SET_TITLE', id, title: 'About — v2' });
expect(next).not.toBe(state);
expect(next.windows[0].title).toBe('About — v2');
});
it('returns the same state for an unknown window id', () => {
const { state } = openOne();
expect(windowReducer(state, { type: 'SET_TITLE', id: 'nope', title: 'x' })).toBe(state);
});
});

View File

@@ -54,8 +54,12 @@ function mapWindow(
let changed = false;
const windows = state.windows.map((win) => {
if (win.id !== id) return win;
changed = true;
return fn(win);
const next = fn(win);
// Only a window that actually came back different counts as a change:
// returning a fresh state object for a no-op update makes every consumer
// of the context re-render, which is enough to spin a render loop.
if (next !== win) changed = true;
return next;
});
return changed ? { ...state, windows } : state;
}