Isolate scroll-driven virtualization updates

docs/layout-rendering.md

@@ -124,15 +124,16 @@ The indent is applied as `transform: translateX(${1.5 - indent}em)` on the inner
 
 ## Virtualization
 
-`LayoutTree` virtualizes the bottom of the list. The virtualization boundary is:
+`LayoutTree` computes `viewportBottomOffset = spaceAbove + singleLineHeight * 5` and passes it with `innerHeight`. Each `TreeNode` combines those stable values with the current `scrollTop` to get the bottom virtualization boundary:
 
 ```ts
-viewportBottom = viewportBottomState (= scrollTop + innerHeight)
+viewportBottom = max(scrollTop, 0)
+               + innerHeight
                + spaceAbove
-               + (singleLineHeight * 5)   // overshoot, so a small scroll doesn't reveal blanks
+               + (singleLineHeight * 5) // overshoot, so a small scroll doesn't reveal blanks
 ```
 
-Thoughts whose `y > viewportBottom` are still in `treeThoughtsPositioned` but rendered with `height: 0` if both `belowCursor` and `!isVisible`. (Above the cursor, the autocrop already takes care of the blank.)
+Each `TreeNode` subscribes to `scrollTopStore` with a selector that returns only whether that thought is beyond the boundary. Most scroll updates leave this boolean unchanged, so they do not rerender `LayoutTree`, `TransitionGroup`, or the full thought list. A `TreeNode` returns `null` only when it is below the cursor, is not the cursor itself, and its `y` is more than one estimated thought height beyond `viewportBottom`. It remains in `treeThoughtsPositioned` so crossing the boundary can render it without rebuilding the list, while the fixed container height keeps the document height stable. (Above the cursor, autocrop already handles the blank space.)
 
 ## `useSizeTracking` and the `sizes` map
 

src/components/LayoutTree.tsx

@@ -1,5 +1,5 @@
 import { isEqual, throttle } from 'lodash'
-import { RefObject, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'
+import { RefObject, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'
 import { useSelector } from 'react-redux'
 import { TransitionGroup } from 'react-transition-group'
 import { css, cx } from '../../styled-system/css'
@@ -14,8 +14,6 @@ import fauxCaretTreeProvider from '../recipes/fauxCaretTreeProvider'
 import { hasChildren } from '../selectors/getChildren'
 import linearizeTree from '../selectors/linearizeTree'
 import nextSibling from '../selectors/nextSibling'
-import reactMinistore from '../stores/react-ministore'
-import scrollTopStore from '../stores/scrollTop'
 import viewportStore from '../stores/viewport'
 import head from '../util/head'
 import parentOf from '../util/parentOf'
@@ -26,12 +24,6 @@ import TreeNode from './TreeNode'
 /** The padding-bottom of the .content element. Make sure it matches the CSS. */
 const CONTENT_PADDING_BOTTOM = 153
 
-/** A computed store that tracks the bottom of the viewport. Used for list virtualization. Does not include overscroll, i.e. if the user scrolls past the top of the document viewportBottom will not change. */
-const viewportBottomStore = reactMinistore.compose(
-  (viewport, scrollTop) => Math.max(scrollTop, 0) + viewport.innerHeight,
-  [viewportStore, scrollTopStore],
-)
-
 /** Calculates the height of a single-line thought. Initially uses an estimated height, then uses the height measured from thn DOM. */
 const useSingleLineHeight = (sizes: Index<{ height: number; width?: number; isVisible: boolean }>) => {
   const fontSize = useSelector(state => state.fontSize)
@@ -215,17 +207,9 @@ const LayoutTree = () => {
     },
   )
 
-  // The bottom of all visible thoughts in a virtualized list where thoughts below the viewport are hidden (relative to document coordinates; changes with scroll position).
-  const viewportBottom = viewportBottomStore.useSelector(
-    useCallback(
-      viewportBottomState => {
-        // the number of additional thoughts below the bottom of the screen that are rendered
-        const overshoot = singleLineHeight * 5
-        return viewportBottomState + spaceAbove + overshoot
-      },
-      [singleLineHeight, spaceAbove],
-    ),
-  )
+  // Offset the virtualization boundary by hidden space above the cursor and five additional thoughts below the
+  // viewport. TreeNode combines this stable offset with scrollTop so that LayoutTree does not rerender on scroll.
+  const viewportBottomOffset = spaceAbove + singleLineHeight * 5
 
   const { footerHeight, navbarHeight } = useNavAndFooterHeight()
   const navAndFooterHeight = navbarHeight + footerHeight
@@ -328,7 +312,8 @@ const LayoutTree = () => {
               thoughtKey={thought.key}
               editing={editing || false}
               {...{
-                viewportBottom,
+                viewportHeight,
+                viewportBottomOffset,
                 treeThoughtsPositioned,
                 bulletWidth,
                 cursorUncleId,

src/components/TreeNode.tsx

@@ -7,6 +7,7 @@ import TreeThoughtPositioned from '../@types/TreeThoughtPositioned'
 import testFlags from '../e2e/testFlags'
 import useFauxCaretNodeProvider from '../hooks/useFauxCaretCssVars'
 import isContextViewActive from '../selectors/isContextViewActive'
+import scrollTopStore from '../stores/scrollTop'
 import isDescendantPath from '../util/isDescendantPath'
 import DropCliff from './DropCliff'
 import FadeTransition from './FadeTransition'
@@ -42,7 +43,8 @@ const TreeNode = ({
   x,
   y,
   index,
-  viewportBottom,
+  viewportHeight,
+  viewportBottomOffset,
   treeThoughtsPositioned,
   bulletWidth,
   cursorUncleId,
@@ -55,7 +57,9 @@ const TreeNode = ({
 }: TreeThoughtPositioned & {
   thoughtKey: string
   index: number
-  viewportBottom: number
+  viewportHeight: number
+  /** The hidden space and overscan added to the bottom of the viewport for list virtualization. */
+  viewportBottomOffset: number
   treeThoughtsPositioned: TreeThoughtPositioned[]
   bulletWidth: number
   cursorUncleId: string | null
@@ -118,13 +122,23 @@ const TreeNode = ({
 
   const onResize: OnResize = useCallback(props => setSize({ ...props, cliff }), [cliff, setSize])
 
+  // Subscribe to the virtualization result rather than scrollTop itself. Most scroll frames leave this boolean
+  // unchanged, so only thoughts that cross the viewport boundary rerender. Keeping the scroll subscription out of
+  // LayoutTree also avoids rerendering TransitionGroup and every TreeNode on each scroll frame.
+  const isBelowViewport = scrollTopStore.useSelector(
+    scrollTop =>
+      belowCursor &&
+      !isCursor &&
+      y > Math.max(scrollTop, 0) + viewportHeight + viewportBottomOffset + singleLineHeightWithCliff,
+  )
+
   // List Virtualization
   // Do not render thoughts that are below the viewport.
   // Exception: The cursor thought and its previous siblings may temporarily be out of the viewport, such as if when New Subthought is activated on a long context. In this case, the new thought will be created below the viewport and needs to be rendered in order for scrollCursorIntoView to be activated.
   // Render virtualized thoughts with their estimated height so that document height is relatively stable.
   // Perform this check here instead of in virtualThoughtsPositioned since it changes with the scroll position (though currently `sizes` will change as new thoughts are rendered, causing virtualThoughtsPositioned to re-render anyway).
   // Use the stable estimated height (singleLineHeightWithCliff) rather than the measured height. Otherwise the cutoff depends on whether the thought is currently mounted: a thought at the fold mounts with the estimate, measures a (smaller) height, flips the cutoff to unmount, which removes its measured size and restores the estimate, re-mounting it. That feedback loop runs entirely within passive effects and triggers "Maximum update depth exceeded" (React error #185). See: https://github.com/cybersemics/em/issues/4270.
-  if (belowCursor && !isCursor && y > viewportBottom + singleLineHeightWithCliff) {
+  if (isBelowViewport) {
     return null
   }
 

src/components/__tests__/LayoutTree.virtualization.ts

@@ -0,0 +1,103 @@
+import { act } from 'react'
+import { importTextActionCreator as importText } from '../../actions/importText'
+import createTestApp, { cleanupTestApp } from '../../test-helpers/createTestApp'
+import dispatch from '../../test-helpers/dispatch'
+import queryThoughtByText from '../../test-helpers/queries/queryThoughtByText'
+import { setCursorFirstMatchActionCreator as setCursor } from '../../test-helpers/setCursorFirstMatch'
+
+const profilerOnRender = vi.fn()
+
+beforeEach(async () => {
+  profilerOnRender.mockClear()
+  await createTestApp({ profilerOnRender })
+})
+
+afterEach(async () => {
+  document.documentElement.scrollTop = 0
+  vi.restoreAllMocks()
+  await cleanupTestApp()
+})
+
+it('does not commit when no thought crosses the viewport boundary while preserving boundary virtualization', async () => {
+  vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue(new DOMRect(0, 0, 100, 36))
+  const innerHeight = vi.spyOn(window, 'innerHeight', 'get').mockReturnValue(72)
+  const resizeHost = window.visualViewport ?? window
+  await act(async () => {
+    resizeHost.dispatchEvent(new Event('resize'))
+    await vi.runOnlyPendingTimersAsync()
+  })
+
+  await dispatch(
+    importText({
+      text: Array.from({ length: 60 }, (_, index) => `- thought ${index + 1}`).join('\n'),
+    }),
+  )
+  await dispatch(setCursor(null))
+  await act(vi.runOnlyPendingTimersAsync)
+
+  // The ninth thought is exactly on the strict virtualization boundary:
+  // 72px viewport + five 36px overscan rows + one 36px row height = 288px.
+  expect(await queryThoughtByText('thought 9')).not.toBeNull()
+  expect(await queryThoughtByText('thought 10')).toBeNull()
+  expect(await queryThoughtByText('thought 60')).toBeNull()
+
+  profilerOnRender.mockClear()
+
+  // Safari reports negative scrollTop values during elastic overscroll. The virtualization boundary must stay
+  // clamped to zero so that rubber-banding at the top does not mount or unmount thoughts.
+  await act(async () => {
+    document.documentElement.scrollTop = -1
+    window.dispatchEvent(new Event('scroll'))
+    await vi.runOnlyPendingTimersAsync()
+  })
+
+  expect(profilerOnRender).not.toHaveBeenCalled()
+  expect(await queryThoughtByText('thought 9')).not.toBeNull()
+  expect(await queryThoughtByText('thought 10')).toBeNull()
+
+  profilerOnRender.mockClear()
+  await act(async () => {
+    document.documentElement.scrollTop = 1
+    window.dispatchEvent(new Event('scroll'))
+    await vi.runOnlyPendingTimersAsync()
+  })
+
+  expect(profilerOnRender).not.toHaveBeenCalled()
+  expect(await queryThoughtByText('thought 9')).not.toBeNull()
+  expect(await queryThoughtByText('thought 10')).toBeNull()
+  expect(await queryThoughtByText('thought 60')).toBeNull()
+
+  innerHeight.mockReturnValue(108)
+  await act(async () => {
+    resizeHost.dispatchEvent(new Event('resize'))
+    await vi.runOnlyPendingTimersAsync()
+  })
+
+  expect(await queryThoughtByText('thought 10')).not.toBeNull()
+  expect(await queryThoughtByText('thought 11')).toBeNull()
+
+  innerHeight.mockReturnValue(72)
+  await act(async () => {
+    resizeHost.dispatchEvent(new Event('resize'))
+    await vi.runOnlyPendingTimersAsync()
+  })
+
+  expect(await queryThoughtByText('thought 10')).toBeNull()
+
+  // Thought 60 crosses its 288px-ahead cutoff around 1836px; 1900px is reachable for this 2160px list.
+  await act(async () => {
+    document.documentElement.scrollTop = 1900
+    window.dispatchEvent(new Event('scroll'))
+    await vi.runOnlyPendingTimersAsync()
+  })
+
+  expect(await queryThoughtByText('thought 60')).not.toBeNull()
+
+  await act(async () => {
+    document.documentElement.scrollTop = 0
+    window.dispatchEvent(new Event('scroll'))
+    await vi.runOnlyPendingTimersAsync()
+  })
+
+  expect(await queryThoughtByText('thought 60')).toBeNull()
+})

src/e2e/puppeteer/__tests__/virtualization-height.ts

@@ -0,0 +1,85 @@
+import clickThought from '../helpers/clickThought'
+import paste from '../helpers/paste'
+import scrollTo from '../helpers/scrollTo'
+import waitForEditable from '../helpers/waitForEditable'
+import { page } from '../session'
+
+vi.setConfig({ testTimeout: 20000, hookTimeout: 20000 })
+
+const DOCUMENT_HEIGHT_SETTLE_TIMEOUT = 5000
+
+/** Waits until the document height is unchanged across consecutive animation frames, then returns it. */
+const waitForDocumentHeightToSettle = () =>
+  page.evaluate(
+    (timeout: number) =>
+      new Promise<number>((resolve, reject) => {
+        const start = performance.now()
+        let heightPrevious = document.documentElement.scrollHeight
+        let stableFrames = 0
+
+        /** Checks the document height again on the next animation frame. */
+        const checkHeight = () => {
+          const height = document.documentElement.scrollHeight
+
+          if (performance.now() - start > timeout) {
+            reject(new Error(`Document height did not settle within ${timeout}ms (last height: ${height}px).`))
+            return
+          }
+
+          stableFrames = height === heightPrevious ? stableFrames + 1 : 0
+          heightPrevious = height
+
+          if (stableFrames === 3) {
+            resolve(height)
+          } else {
+            requestAnimationFrame(checkHeight)
+          }
+        }
+
+        requestAnimationFrame(checkHeight)
+      }),
+    DOCUMENT_HEIGHT_SETTLE_TIMEOUT,
+  )
+
+it('restores the document height after a wrapped thought is virtualized again', async () => {
+  const wrappedThought =
+    'This wrapped thought is intentionally long enough to occupy many rendered lines. Its measured height must be removed after it leaves the virtualization window so that it cannot leave phantom scroll space behind. This sentence adds more width to make the difference from the single-line estimate large and unambiguous in a real browser.'
+  const shortThoughts = Array.from({ length: 30 }, (_, index) => `      - thought ${index + 1}`).join('\n')
+
+  await paste(`
+    - parent
+${shortThoughts}
+      - ${wrappedThought}
+    - last
+  `)
+  await clickThought('parent')
+  await waitForEditable('thought 1')
+  await scrollTo(0, 0)
+  await page.waitForFunction(
+    (value: string) =>
+      !Array.from(document.querySelectorAll('[data-editable]')).some(element => element.textContent === value),
+    {},
+    wrappedThought,
+  )
+
+  const initialHeight = await waitForDocumentHeightToSettle()
+
+  // Scroll with real wheel input so the wrapped thought enters the virtualization window and is measured by the browser.
+  await page.mouse.wheel({ deltaY: initialHeight })
+  await waitForEditable(wrappedThought)
+  const measuredHeight = await waitForDocumentHeightToSettle()
+  expect(measuredHeight).toBeGreaterThan(initialHeight)
+
+  // Scroll back with real wheel input so the wrapped thought is virtualized and its measured height is discarded.
+  await page.mouse.wheel({ deltaY: -measuredHeight })
+  await page.waitForFunction(
+    (value: string) =>
+      window.scrollY === 0 &&
+      !Array.from(document.querySelectorAll('[data-editable]')).some(element => element.textContent === value),
+    {},
+    wrappedThought,
+  )
+
+  const restoredHeight = await waitForDocumentHeightToSettle()
+  expect(restoredHeight).toBe(initialHeight)
+})

src/hooks/__tests__/useSizeTracking.ts

@@ -0,0 +1,70 @@
+import { renderHook } from '@testing-library/react'
+import { act } from 'react'
+import ThoughtId from '../../@types/ThoughtId'
+import useSizeTracking from '../useSizeTracking'
+
+it('removes only the tracked size', () => {
+  const { result } = renderHook(useSizeTracking)
+
+  act(() => {
+    result.current.setSize({
+      cliff: 0,
+      height: 20,
+      id: 'thought-1' as ThoughtId,
+      isVisible: true,
+      key: 'thought-1',
+    })
+    result.current.setSize({
+      cliff: 0,
+      height: 30,
+      id: 'thought-2' as ThoughtId,
+      isVisible: true,
+      key: 'thought-2',
+    })
+  })
+
+  const sizesBeforeRemoval = result.current.sizes
+
+  act(() => {
+    result.current.setSize({
+      cliff: 0,
+      height: null,
+      id: 'thought-1' as ThoughtId,
+      isVisible: true,
+      key: 'thought-1',
+    })
+  })
+
+  expect(result.current.sizes).not.toBe(sizesBeforeRemoval)
+  expect(result.current.sizes).toEqual({
+    'thought-2': sizesBeforeRemoval['thought-2'],
+  })
+})
+
+it('preserves the sizes map when the key is already absent', () => {
+  const { result } = renderHook(useSizeTracking)
+
+  act(() => {
+    result.current.setSize({
+      cliff: 0,
+      height: 20,
+      id: 'thought-1' as ThoughtId,
+      isVisible: true,
+      key: 'thought-1',
+    })
+  })
+
+  const sizesBeforeRemoval = result.current.sizes
+
+  act(() => {
+    result.current.setSize({
+      cliff: 0,
+      height: null,
+      id: 'thought-2' as ThoughtId,
+      isVisible: true,
+      key: 'thought-2',
+    })
+  })
+
+  expect(result.current.sizes).toBe(sizesBeforeRemoval)
+})