`1ed01e3`

fix: hoist useEffect above early return (rules of hooks v2)

Authored by

espadonne 1 month ago

SHA: 1ed01e3c4be1eb9e3c42208509dc1c2daafc68cd
Parents: d47e25d
Tree: a75e3d0

1 changed file

Status	File	+	-
M	`src/components/ViewerPane.tsx`	32	42

src/components/ViewerPane.tsxmodified

      return false;
    }, [detail, inFlightTurns]);
 -  // Build the claude argv for terminal mode. Must live above the
 -  // early returns below — React's rules of hooks require every
 -  // hook to run in the same order on every render, so an early
 -  // return followed by a later `useMemo` triggers the
 -  // "Rendered more hooks than during the previous render" crash.
 +  // ALL HOOKS MUST RUN BEFORE ANY EARLY RETURN. React's rules of
 +  // hooks require a fixed call order per render, and an early
 +  // return sitting above a later hook violates it with
 +  // "Rendered more hooks than during the previous render".
    const claudeArgs = useMemo<string[]>(() => {
      if (!headerSummary) return [];
      if (headerSummary.source === "archive") return [];
      return ["--resume", headerSummary.id];
    }, [headerSummary]);
 +  // Lazy detail load for terminal-default sessions that the user
 +  // has just toggled to cards mode. Computed pre-return so the
 +  // hook order stays stable whether or not headerSummary is set.
 +  const lazyLoadTarget = useMemo<SessionSummary | null>(() => {
 +    if (!headerSummary) return null;
 +    if (headerSummary.source === "archive") return null;
 +    const mode = viewerMode.get(headerSummary.id) ?? "cards";
 +    if (mode !== "cards") return null;
 +    if (loading) return null;
 +    if (detail && detail.summary.id === headerSummary.id) return null;
 +    return headerSummary;
 +  }, [headerSummary, viewerMode, loading, detail]);
 +  useEffect(() => {
 +    if (!lazyLoadTarget) return;
 +    void ensureDetailFor(lazyLoadTarget);
 +  }, [lazyLoadTarget, ensureDetailFor]);
++
    if (!headerSummary) {
      return (
        <Shell subtitle="" loading={false}>
      );
+   }
 -  const showingSkeleton = pendingSummary !== null && detail === null;
+-
    const summary = headerSummary;
    const messages = detail?.messages ?? [];
    const isArchive = summary.source === "archive";
    const mode = viewerMode.get(summary.id) ?? "cards";
    const hasLivePty = ptyIds.has(summary.id);
 -  // Lazy-load detail if the user is in cards mode for a session
 -  // whose body we skipped fetching on click (because it defaulted
 -  // to terminal mode). Only fires when the detail we need genuinely
 -  // isn't already loaded or in flight.
 -  const needsLazyCardsLoad =
 -    mode === "cards" &&
 -    !isArchive &&
 -    !loading &&
 -    (detail === null || detail.summary.id !== summary.id);
 -  useEffect(() => {
 -    if (!needsLazyCardsLoad) return;
 -    void ensureDetailFor(summary);
 -  }, [needsLazyCardsLoad, summary, ensureDetailFor]);
 +  // Only show the indeterminate stripe when we're showing
 +  // *cached* content while a background refresh runs. On a cold
 +  // load the CardsSkeleton body is its own loading indicator, so
 +  // a second stripe on top of it would just be noise.
 +  const refreshing =
 +    loading && detail !== null && detail.summary.id === summary.id;
    return (
      <Shell
        subtitle={summary.title}
 -      loading={loading}
 +      loading={refreshing}
        right={
          <div className="flex items-center gap-2 text-[10px] text-fg-3">
            {isArchive ? (
+     >
        {!isArchive && mode === "terminal" && summary.cwd ? (
          // key forces a fresh mount per session — xterm state is
 -        // bound to sessionId so switching sessions must teardown
 -        // and re-open (the backend PTY keeps running regardless).
 -        //
 -        // Suspense fallback renders while the lazy chunk downloads
 -        // on first use. Once loaded, the chunk is cached so
 -        // subsequent mounts are instant.
 +        // bound to sessionId so the backend PTY keeps running
 +        // regardless.
          <Suspense fallback={<TerminalLoading />}>
            <TerminalPane
              key={summary.id}
              claudeArgs={claudeArgs}
            />
          </Suspense>
 -      ) : showingSkeleton ? (
 +      ) : !detail || detail.summary.id !== summary.id ? (
          <CardsSkeleton />
 -      ) : detail ? (
 +      ) : (
          <div className="flex h-full flex-col">
            <div className="min-h-0 flex-1">
              <MessageTimeline messages={messages} autoFollow={hasActiveTurn} />
              <ChatInput detail={detail} />
            )}
          </div>
 -      ) : (
 -        <CardsSkeleton />
        )}
      </Shell>
    );
+ }
 -/** Tiny placeholder for the lazy TerminalPane chunk. Only visible
 - *  on the first-ever terminal mount in a session — subsequent
 - *  mounts reuse the cached module and render instantly. */
  function TerminalLoading() {
    return (
      <div className="flex h-full items-center justify-center text-[11px] text-fg-3">
    );
+ }
 -/** Shimmer placeholder shown while a session detail is loading.
 - *  Six ghost message cards, sized similarly to real messages, with
 - *  a subtle animation. Keeps the viewer feeling alive instead of
 - *  showing a frozen "loading…" string. */
  function CardsSkeleton() {
    return (
      <div className="flex h-full flex-col gap-4 overflow-hidden p-4">
      <div className="flex h-full flex-col overflow-hidden">
        <PaneHeader title="Viewer" subtitle={subtitle} right={right} />
        {loading && (
 -        // Indeterminate progress stripe pinned under the header.
 -        // Tells the user "something is in flight" without the
 -        // jarring "loading session…" centered text that used to
 -        // replace the whole body on every click.
          <div
            aria-hidden
            className="h-0.5 w-full shrink-0 bg-gradient-to-r from-transparent via-accent/60 to-transparent"