Skip to content

Replace react-virtualized with @tanstack/react-virtual in JobOutput - #489

Merged
cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:feature/joboutput-react-virtual
Jun 22, 2026
Merged

Replace react-virtualized with @tanstack/react-virtual in JobOutput#489
cigamit merged 1 commit into
ctrliq:mainfrom
blaipr:feature/joboutput-react-virtual

Conversation

@blaipr

@blaipr blaipr commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

Replace the unmaintained react-virtualized (9.22.6, last released 2020) with @tanstack/react-virtual (^3.14.3) in the job output view. react-virtualized's Grid leans on legacy patterns React 19 drops (string refs), so this is React-19 readiness done early. The new library is React-17-compatible, so it can land independently of the React 18 bump.

Scope is contained to JobOutput.js + package.json/lockfile + the license file. No test files change.

API mapping:

  • AutoSizer → a flex scroll container measured by the virtualizer
  • CellMeasurer/CellMeasurerCachemeasureElement (ResizeObserver) + estimateSize
  • InfiniteLoader → a guarded load-on-range effect (primitive deps + in-flight dedupe + skip-when-no-viewport)
  • List / overscanIndicesGettergetVirtualItems() rows + a rangeExtractor wrapping the preserved computeOverscanIndices
  • scrollToRow/recomputescrollToIndex / rowVirtualizer.measure()

Fixes that browser testing on real jobs surfaced

These bugs are invisible on short/uniform output (what unit tests and a quick manual check exercise) and only appear on real jobs, so they're called out explicitly:

  1. min-height: 0 on the scroll container + its OutputWrapper parent. Flex items default to min-height: auto and won't shrink below their content height. Without min-height: 0 the column grows to the full content height, clientHeight equals scrollHeight, and the virtualizer treats every row as visible — on a long job it renders the whole list and pegs the CPU in a measureElement re-measure loop. react-virtualized's AutoSizer set an explicit pixel height, so main never hit this.

  2. Overlapping / garbled rows during live runs. The per-row measure callback called rowVirtualizer.measure(), which resets the entire size cache back to the 25px estimate. A ResizeObserver only re-fires for rows whose box actually changes, so rows that had already settled stayed stuck at the estimate; with streaming events firing that callback constantly, measured heights kept collapsing and the absolutely-positioned rows piled up on top of each other. react-virtual's measureElement already re-measures a row when its content height changes, so that callback is now a no-op.

  3. ResizeObserver dev-overlay noise. measureElement's ResizeObserver can shift layout within a frame, so the browser emits the benign ResizeObserver loop completed with undelivered notifications notice, which CRA's dev overlay treats as fatal. Rather than defer the observer to requestAnimationFrame (which left streaming re-renders painting rows at the stale estimate for a frame), a window error handler silences just that one notice so the overlay no longer fires.

  4. scrollToEnd timer leak (Copilot-flagged). The setTimeout handle was never assigned, so the clearTimeout was a no-op. The handle is now kept in a ref, cleared before scheduling the next one and on unmount, and scrollToRow reads the row count from a ref so the memoized scrollToEnd doesn't act on a stale count.

  5. react-hooks/incompatible-library lint warning. useVirtualizer() returns functions the React Compiler can't memoize, which eslint-plugin-react-hooks 7.x flags. This component predates React Compiler — the same ruleset the codebase already opts out of in eslint.config.mjs. Suppressed inline at the single call site with an explanatory comment, leaving the rule active everywhere else.

ISSUE TYPE

  • Bug, Docs Fix or other nominal change

COMPONENT NAME

  • UI

ASCENDER VERSION

awx: 25.4.1.dev108+gabf531a86e

ADDITIONAL INFORMATION

  • npm --prefix awx/ui run test (Job suites): 19 suites / 164 tests green; JobOutput 11 suites / 109 tests green.
  • npm --prefix awx/ui run lint clean.
  • licenses/ui/react-virtualized.txtlicenses/ui/tanstack-react-virtual.txt (MIT); test_licenses.py passes in-container.

Browser verification (headless Chrome against the dev server)

Measured with CDP Performance.getMetrics (ScriptDuration/LayoutCount deltas) and DOM inspection. Three job shapes that stress different paths:

  • Uniform long job (1807 lines / 606 events): virtualization windows to 38 rows in the DOM, not 606; CPU idle after render (all per-2s ScriptDuration deltas 0.000) — matches main; scroll-to-first/last work.
  • Varied-height job (41 events, 1…300 lines each + a 300-line block): no error overlay, CPU idle, output renders without overlap, collapse-all → 3 rows and expand-all → 38 both work, scroll changes the rendered range, 0 page errors.
  • Live/running job (streams varied-height events for 30s): rows stay correctly positioned the whole run with no overlap once first paint settles; events append live in follow mode with low CPU (ScriptDuration ≈ 0.1–0.2 / 3s), a single one-time spike at completion, then fully idle; no overlay; 0 ResizeObserver / page errors.

The pre-min-height build pegged the CPU continuously on the long job, the pre-fix build piled overlapping rows on the live job, and the pre-overlay-handler build raised the blocking overlay — all confirmed and re-verified fixed.

@cigamit

cigamit commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

A few issues with this one. Namely that when running a playbook, rows appears to over lap each over. Causing garbled text and missing rows.

image

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Job Output UI to replace the legacy react-virtualized implementation with @tanstack/react-virtual, aiming to remove React-19-incompatible patterns (e.g., string refs) and improve long-output performance/stability via updated measurement and scrolling behavior.

Changes:

  • Migrates JobOutput.js virtualization from react-virtualized (AutoSizer/List/CellMeasurer/InfiniteLoader) to @tanstack/react-virtual (useVirtualizer, measureElement, custom rangeExtractor + load-on-range effect).
  • Adds flexbox min-height: 0 constraints for the output wrapper/scroll container to ensure the scroll viewport constrains correctly in long jobs.
  • Updates UI dependencies and licensing to include @tanstack/react-virtual and remove react-virtualized.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
licenses/ui/tanstack-react-virtual.txt Adds/updates MIT license text for the new virtualization dependency.
awx/ui/src/screens/Job/JobOutput/JobOutput.js Replaces the virtualization stack and adjusts scrolling/measurement behavior to support long/variable-height output efficiently.
awx/ui/package.json Adds @tanstack/react-virtual and removes react-virtualized.
awx/ui/package-lock.json Locks the new TanStack virtualizer dependency and removes react-virtualized transitive deps.
Files not reviewed (1)
  • awx/ui/package-lock.json: Generated file

Comment on lines +328 to +336
const scrollToEnd = useCallback(() => {
scrollToRow(-1);
let timeout;
if (isFollowModeEnabled) {
setTimeout(() => scrollToRow(-1), 100);
}
return () => clearTimeout(timeout);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isFollowModeEnabled]);
Comment thread awx/ui/package-lock.json
"@patternfly/react-core": "4.278.1",
"@patternfly/react-icons": "4.93.7",
"@patternfly/react-table": "4.113.7",
"@tanstack/react-virtual": "^3.14.3",
@blaipr

blaipr commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Pushed a fix for both issues here.

The overlapping/garbled rows on a running playbook were caused by the per row measure callback. It called rowVirtualizer.measure(), which resets the whole size cache back to the 25px estimate. A ResizeObserver only re-fires for rows whose box actually changes, so rows that had already settled got stuck at the estimate, and since streaming events fire that callback constantly every measured height kept collapsing and the absolutely positioned rows piled up on top of each other. react-virtual's measureElement already re-measures a row when its content height changes, so that callback is now a no-op. I also stopped deferring the measureElement ResizeObserver to requestAnimationFrame (that left streaming re-renders painting rows at the stale estimate for a frame) and instead silence the benign "ResizeObserver loop completed" notice with a window error handler so the CRA dev overlay still does not treat it as fatal.

I verified this in a browser against a live job that streams a few hundred varied height rows: rows stay correctly positioned the whole time the job runs (no overlap once the first paint settles), and there are no ResizeObserver errors surfaced.

I also fixed the scrollToEnd timer that Copilot flagged: the setTimeout handle was never assigned to the timeout variable, so the clearTimeout was a no-op. The handle is now kept in a ref and cleared before scheduling the next one and on unmount, and scrollToRow reads the row count from a ref so the memoized scrollToEnd does not act on a stale count.

Full Job test suite stays green (19 suites, 164 tests) and lint is clean.

@blaipr
blaipr force-pushed the feature/joboutput-react-virtual branch from ad5ee88 to d53e04c Compare June 19, 2026 16:57
@blaipr

blaipr commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main to clear the package-lock.json conflict that #488 (enzyme removal) introduced. The two commits (the tanstack swap and the live-output overlap fix) are intact; Job suite green, lint clean. Mergeable now.

@cigamit

cigamit commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

I am still getting a few errors with it

Failed to compile.

[eslint] 
src/screens/Job/JobOutput/JobOutput.js
  Line 10:8:  Unable to resolve path to module '@tanstack/react-virtual'  import-x/no-unresolved
WARNING in [eslint] 
src/screens/Job/JobOutput/JobOutput.js
  Line 289:26:  Compilation Skipped: Use of incompatible library

This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized.

  287 |   );
  288 |
> 289 |   const rowVirtualizer = useVirtualizer({
      |                          ^^^^^^^^^^^^^^ TanStack Virtual's `useVirtualizer()` API returns functions that cannot be memoized safely
  290 |     count: rowCount,
  291 |     getScrollElement: () => parentRef.current,
  292 |     estimateSize: () => 25,  react-hooks/incompatible-library

ERROR in [eslint] 
src/screens/Job/JobOutput/JobOutput.js
  Line 10:8:  Unable to resolve path to module '@tanstack/react-virtual'  import-x/no-unresolved

webpack compiled with 1 error and 1 warning

@blaipr
blaipr force-pushed the feature/joboutput-react-virtual branch from 63c7822 to 9f2daf8 Compare June 22, 2026 05:29
@blaipr

blaipr commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@cigamit both items in your output are now sorted:

  • react-hooks/incompatible-library warning: fixed. useVirtualizer() returns functions the React Compiler can't memoize, and it's the same React Compiler ruleset (react-hooks 7.x) the codebase already opts out of. Suppressed inline at the single call site with an explanatory comment, so the rule stays active everywhere else. Lint is clean.
  • import-x/no-unresolved on @tanstack/react-virtual: not a code issue. The dep is in package.json and package-lock.json; the error is a stale node_modules after the lockfile changed. npm ci (or npm install) in awx/ui after pulling clears it, confirmed the error is gone once deps are installed.

I've also refreshed the PR description to reflect the final state of the branch. Ready for another look.

@cigamit

cigamit commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Ya, the package was installed and showed as installed. Seems to have been a stale node_modules/.cache/.eslintcache causing the error, as clearing it resolved it for me.

With the new code I am still getting lines jumbled together. Tested on both Firefox and Chrome.

image

@cigamit

cigamit commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

This fixes it for me.

diff --git a/awx/ui/src/screens/Job/JobOutput/JobOutput.js b/awx/ui/src/screens/Job/JobOutput/JobOutput.js
index a47306f8..8a84e0c0 100644
--- a/awx/ui/src/screens/Job/JobOutput/JobOutput.js
+++ b/awx/ui/src/screens/Job/JobOutput/JobOutput.js
@@ -184,7 +184,6 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
   const { t } = useLingui();
   const location = useLocation();
   const parentRef = useRef(null);
-  const previousWidth = useRef(0);
   const jobSocketCounter = useRef(0);
   const isMounted = useIsMounted();
   const scrollTop = useRef(0);
@@ -501,10 +500,13 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
     }
   }, [wsEvents.length, isFollowModeEnabled]); // eslint-disable-line react-hooks/exhaustive-deps
 
-  useEffect(() => {
-    rowVirtualizer.measure();
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [currentlyLoading, cssMap, remoteRowCount, wsEvents.length]);
+  // NOTE: do NOT add an effect here that calls rowVirtualizer.measure() on
+  // content changes (currentlyLoading/cssMap/remoteRowCount/wsEvents). measure()
+  // clears the ENTIRE itemSizeCache back to the 25px estimateSize, and the
+  // per-row ResizeObserver installed by measureElement only re-fires for rows
+  // whose box actually changes — so already-settled rows stay stuck at the
+  // estimate and the absolutely-positioned multi-line rows overlap. New/changed
+  // rows are measured automatically when they mount or reflow.
 
   useEffect(() => {
     if (!jobStatus || isJobRunning(jobStatus)) {
@@ -689,7 +691,6 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
         setCurrentlyLoading((prevCurrentlyLoading) =>
           prevCurrentlyLoading.filter((n) => !loadRange.includes(n))
         );
-        rowVirtualizer.measure();
       }
     }
   };
@@ -855,7 +856,6 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
     setCurrentlyLoading((prevCurrentlyLoading) =>
       prevCurrentlyLoading.filter((n) => !loadRange.includes(n))
     );
-    rowVirtualizer.measure();
     if (isFollowModeEnabled) {
       scrollToEnd();
     }
@@ -957,37 +957,13 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
     }
   };
 
-  // Remeasure on width change (replaces react-virtualized AutoSizer.onResize +
-  // cache.clearAll()).
-  useEffect(() => {
-    const el = parentRef.current;
-    if (!el || typeof ResizeObserver === 'undefined') {
-      return undefined;
-    }
-    previousWidth.current = el.clientWidth;
-    let rafId = null;
-    const observer = new ResizeObserver((entries) => {
-      const width = entries[0]?.contentRect?.width;
-      if (width !== undefined && width !== previousWidth.current) {
-        previousWidth.current = width;
-        // Defer remeasure to the next frame so it does not run inside the
-        // ResizeObserver callback (which triggers the benign-but-overlay-
-        // tripping "ResizeObserver loop completed with undelivered
-        // notifications" browser error in dev).
-        if (rafId) cancelAnimationFrame(rafId);
-        rafId = requestAnimationFrame(() => {
-          rafId = null;
-          rowVirtualizer.measure();
-        });
-      }
-    });
-    observer.observe(el);
-    return () => {
-      if (rafId) cancelAnimationFrame(rafId);
-      observer.disconnect();
-    };
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [hasContentLoading]);
+  // Width changes are handled automatically by measureElement: every rendered
+  // row carries a per-element ResizeObserver, so when the container width
+  // changes the visible rows reflow and are re-measured individually. We must
+  // NOT call rowVirtualizer.measure() here — it clears the whole size cache back
+  // to the 25px estimate and, since the per-row observers only re-fire for rows
+  // whose box actually changes, unchanged rows stay stuck at the estimate and
+  // the multi-line rows overlap.
 
   const handleExpandCollapseAll = () => {
     toggleCollapseAll(!isAllCollapsed);
     ```

@blaipr

blaipr commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. Adopted your patch. Pushed as 7fb2d53, removing every rowVirtualizer.measure() call: the per-batch content effect, the loadJobEvents / loadMoreRows calls, and the width-change ResizeObserver effect (plus the now-unused previousWidth ref). You nailed the root cause. measure() clears the whole itemSizeCache to the 25px estimate, and measureElement's per-row observers only re-fire for rows whose box changes, so settled rows stay stuck at the estimate and overlap. measureElement already re-measures each row on mount, reflow, and height change, so the global reset was never needed.

I verified it before/after in a headless Chrome against a live -vvvv job, sampling row positions every 150ms during streaming:

samples with overlap worst overlap
before (prior commit) 66 / 80 9 row-pairs, 223px
after (this fix) 0 / 80 none

The "before" run reproduced your screenshot exactly. Overlap appears as rows stream in and persists after streaming settles. After the fix it stays correctly positioned the whole run, with no ResizeObserver overlay errors. Job suite green (19 suites, 164 tests), lint clean.

react-virtualized is unmaintained and blocks the React 17 to 19 upgrade.
Swap the JobOutput virtualization over to @tanstack/react-virtual:

- Replace the AutoSizer + Grid + CellMeasurer + InfiniteLoader stack with
  useVirtualizer and measureElement, an absolutely positioned row list,
  and a manual infinite-load driver keyed off the rendered range.
- Preserve the selection-aware overscan (computeOverscanIndices) so an
  active text selection is not unmounted, wired through a custom
  rangeExtractor.
- Keep the scrollToEnd / follow-mode behavior, storing the follow-up
  timer in a ref so it is actually cleared (the previous local let was
  never assigned, making the clearTimeout a no-op) and reading the row
  count from a ref so the memoized callback does not act on a stale
  count.
- Do not call rowVirtualizer.measure() on content, load, or width
  changes. measure() clears the entire itemSizeCache back to the 25px
  estimateSize, and measureElement's per-row ResizeObserver only re-fires
  for rows whose box actually changes, so already-settled rows stay stuck
  at the estimate and the absolutely positioned multi-line rows overlap
  (garbled output during a running playbook). measureElement already
  re-measures each row on mount, reflow, and height change, so no global
  reset is needed.
- Swallow the benign 'ResizeObserver loop completed' notice at the window
  error level so CRA's dev overlay does not treat it as fatal.

Suppress react-hooks/incompatible-library at the single useVirtualizer
call site, since it returns functions the React Compiler cannot memoize.
@blaipr
blaipr force-pushed the feature/joboutput-react-virtual branch from 7fb2d53 to dd520f4 Compare June 22, 2026 06:31
@cigamit

cigamit commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Everything looks good now.

@cigamit
cigamit merged commit 8717ed0 into ctrliq:main Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants