Skip to content

Fixes 31156: release TipTap editors on unmount and load the ELK layout engine on demand - #31157

Merged
siddhant1 merged 5 commits into
mainfrom
long-animation-frame-block
Aug 10, 2026
Merged

Fixes 31156: release TipTap editors on unmount and load the ELK layout engine on demand#31157
siddhant1 merged 5 commits into
mainfrom
long-animation-frame-block

Conversation

@siddhant1

@siddhant1 siddhant1 commented Aug 7, 2026

Copy link
Copy Markdown
Member

Fixes #31156

Two independent main-thread fixes found while investigating Sentry ui.long-animation-frame spans ("Main UI thread blocked") — 826 spans over two weeks, p99 1461ms, max 4304ms, 153.4 s of total blocking.


1. TipTap editors were never destroyed

useCustomEditor creates a ProseMirror Editor in an effect, but cleanup only flipped a local isMounted flag — instance.destroy() was never called (a grep across src/ matches only test files). The view, plugin stack, contenteditable DOM and MutationObserver outlived the component.

It compounds because the editor mounts once per table row — the shared description column (utils/TableColumn.util.tsx) renders RichTextEditorPreviewerNew, which mounts a BlockEditor.

The isDestroyed guard mirrors @tiptap/react's own useEditor cleanup (node_modules/@tiptap/react/dist/index.js:1013, :1039) — <StrictMode> double-invokes effects, so cleanup can run twice.

Verified at runtime

Production build served against a live backend, driving SPA navigation (pushState + popstate) so components unmount without a page reload — a full reload resets the count regardless and proves nothing:

round 0  tags: 16
round 1  my-data: 9    tags: 16
round 2  my-data: 9    tags: 16
round 3  my-data: 9    tags: 16
round 4  my-data: 9    tags: 16

.ProseMirror count returns to exactly the row count every round and drops 16 → 9 on leaving. No compounding.


2. ELK layout engine loaded on every authenticated page

EntityLineageLayoutUtils imported ELKUtil at module scope. That module also exports five pure geometry helpers, so importing any one of them dragged elkjs — 1.37 MB built.

LineageControlButtons imports only centerNodePosition, and is reached statically from Lineage.component.tsx. That was enough to pull elkjs into the shared chunk that 476 route chunks statically import. Every authenticated page downloaded and parsed the lineage layout engine — including glossary, which uses G6/antv-dagre and never calls ELK (OntologyExplorer chunk: 0 occurrences of org.eclipse.elk).

ELKLayout is referenced in exactly one place, inside getELKLayoutedElements, which is already async and already awaited at every call site. Moving the import inside it is behaviour-preserving; the ELK engine itself is untouched.

Measured on a production build

before after
ServicesPage must-load closure 13.13 MB 11.75 MB
glossary terms tab (runtime) 12.57 MB / 203 chunks 11.20 MB / 202 chunks
ELKUtil chunk importers 1 static 0 static, 1 dynamic

−1.37 MB on every authenticated route.

Verified against a live backend

  • Glossary terms tab: 0 elk-related requests, page still renders (7 rows, 7 editors)
  • /lineage: fetches ELKUtil + vendor-elk on demand, renders 83 nodes, 0 console errors


3. Destroying the editor exposed two unguarded deferred writes

Fix 1 above turned a latent bug into a live one. BlockEditor defers two pieces of
work onto the editor with setTimeout (to dodge a known TipTap flushSync
warning), with no clearTimeout and no re-check inside the callback:

  • the content sync (setEditorContent)
  • the editable toggle (setEditable)

Both effects check editor.isDestroyed at the top, but the timer fires later.
Before this PR the editor was never destroyed, so a stale timer harmlessly wrote
into an orphaned-but-alive editor. Once the instance is destroyed on unmount, the
same timer reaches a torn-down ProseMirror view:

TypeError: Cannot read properties of null (reading 'updateOuterDeco')
  at EditorView.updateStateInner (prosemirror-view)
  at updateState (src/utils/BlockEditorUtils.ts:195)
  at src/components/BlockEditor/BlockEditor.tsx:235

This is reachable in the app, not just in tests: any description cell whose
content or editable prop changes and then unmounts within the same tick —
pagination click, tab switch, search keystroke, autosave round-trip — throws from
a timer callback, outside React, where no error boundary catches it. That is
exactly the high-churn table this PR targets.

Both sites now clear the timer on cleanup and re-check isDestroyed inside the
callback. Clearing also drops a stale content write that could otherwise land
after a newer one.

Caught by CI, confirmed by A/B

ui-coverage-tests went red on EntitySummaryPanel and TestDefinitionList,
both with that identical stack, while the same job is green on every recent main
run. A/B on one stack, changing only the destroy() hunk:

TestDefinitionList result
with destroy(), without the guard 4 failed / 31 passed
with destroy() reverted 35 passed
with destroy() + the guard 35 passed

A sweep of the UI for other post-destroy access paths found none: only three files
combine a TipTap editor with deferred work — BlockEditor (fixed),
useCustomEditor (its rAF is already isMounted-guarded), and FeedEditor
(Quill, not TipTap). No extension NodeView defers editor access.


Type of change

  • Bug fix (non-breaking)
  • Performance improvement (no functional change)

Tests

128 tests pass across the BlockEditor tree and the two suites that caught the
regression (TestDefinitionList, BlockEditor/**), plus the lineage areas
(EntityLineageLayoutUtils, Lineage, EntityLineage, LineageProvider).

useCustomEditor's own suite now covers the unmount path. Its MockEditor had
neither destroy() nor isDestroyed, so the new branch threw
TypeError: instance.destroy is not a function on every unmount — React swallowed
it and the suite still reported green, which is why the first pass of this PR
missed the regression above. The mock now implements both, and a new test asserts
the instance is destroyed on unmount; it is non-vacuous (it fails against main's
hook and passes against this branch's).

Suites that fail to run locally with "Jest encountered an unexpected token" are a
pre-existing environment issue with the ui-core-components prebuilt dist; they
run fine on CI. I A/B'd them with the change removed and they fail identically.

tsc --noEmit: 0 errors in all changed files. prettier --check: clean.

Not verified: ESLint could not run locally — this branch's eslint.config.mjs requires eslint-plugin-sonarjs, absent from the checkouts available here. Relying on CI.

Manual test steps

Editor leak: open a table of descriptions (/tags/General, a glossary terms tab), run document.querySelectorAll('.ProseMirror').length, navigate away and back within the app (don't reload), re-run. Before: grows. After: returns to the row count.

ELK: open a glossary page with DevTools Network filtered to elk — no requests. Then open /lineageELKUtil and vendor-elk load, and the graph renders.

Checklist

  • Self-reviewed
  • Verified at runtime against a live backend, not just unit tests
  • Confirmed no test regressions via control run and per-change A/B
  • UI screen recording — N/A, no visual change

🤖 Generated with Claude Code


Summary by Gitar

  • Performance:
    • Dynamically import ELKUtil in EntityLineageLayoutUtils to prevent bundling elkjs (~1.37MB) across all authenticated pages

This will update automatically on new commits.

…ulating

`useCustomEditor` creates a ProseMirror `Editor` in an effect but its cleanup
only flipped an `isMounted` flag — `instance.destroy()` was never called
(grep finds it only in test files). The editor's view, plugin state,
contenteditable DOM and MutationObserver therefore outlive the component.

This compounds because the editor is mounted per table row: the description
column renders `RichTextEditorPreviewerNew` for every row, so a page of
descriptions is a page of editors. Measured live on release-2-0: 16 live
`.ProseMirror` instances for 16 rows on `/tags/General`, 7 for 7 on a glossary
terms table. Every pagination click, search keystroke or tab switch abandoned
that many editors for the rest of the session.

The `isDestroyed` guard mirrors `@tiptap/react`'s own `useEditor` cleanup
(node_modules/@tiptap/react/dist/index.js:1013 and :1039) — StrictMode
double-invokes effects, so the cleanup can run twice.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 7, 2026 06:16
@siddhant1
siddhant1 requested a review from a team as a code owner August 7, 2026 06:16

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions github-actions Bot added the UI UI specific issues label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 657c41c6fd6cceda887250a60341d50188e68193 in Playwright run 31353694878, attempt 1.

✅ 550 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 50m 59s

⏱️ Max setup 3m 52s · max shard execution 17m 50s · max shard-job elapsed before upload 21m 13s · reporting 4s

🌐 199.86 requests/attempt · 2.83 app boots/UI scenario · 21.03% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 21.03% (convergence target: at most 15%).
  • Application boot ratio was 2.83 per UI scenario (1617 boots / 571 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 141 0 0 0 0 0
✅ Shard chromium-02 140 0 0 0 0 0
✅ Shard chromium-03 118 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

Copilot AI review requested due to automatic review settings August 7, 2026 08:05
@siddhant1 siddhant1 changed the title Fixes 31156: destroy the TipTap editor on unmount so instances stop accumulating Fixes 31156: release TipTap editors on unmount and load the ELK layout engine on demand Aug 7, 2026

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@siddhant1 siddhant1 added safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch labels Aug 7, 2026
…demand

`EntityLineageLayoutUtils` mixed two concerns: five pure geometry helpers
(`centerNodePosition`, `getNodesBoundsReactFlow`, `getViewportForBoundsReactFlow`,
...) and two ELK-backed layout functions. Because `ELKUtil` was imported at
module scope, importing any of the pure helpers dragged elkjs — 1.37MB built.

`LineageControlButtons` imports only `centerNodePosition`, and `LineageProvider`
is statically imported by eight lineage components. That was enough to land
elkjs in the shared chunk which 476 route chunks statically import, so every
authenticated page downloaded and parsed the lineage layout engine. Glossary
pages paid for it while rendering with G6/antv-dagre, never calling ELK at all.

`getELKLayoutedElements` and `positionNodesUsingElk` now live in
`utils/Lineage/Layout/ElkLayoutUtils.ts`, alongside a named `loadElkLayout()`
lazy boundary. `EntityLineageLayoutUtils` no longer references ELK in any form,
so the pure helpers cannot re-acquire the dependency by accident.

A plain file split would not have been enough on its own: `LineageProvider` is
the sole caller of the ELK functions and is itself reachable from the shared
cluster, so the engine needs the dynamic boundary regardless. Making it a named
function in an ELK-specific module keeps that explicit rather than burying an
`await import()` in the middle of layout logic.

Measured on a production build:

  ELKUtil chunk importers   1 static -> 0 static, 1 dynamic
  glossary terms tab        12.57 MB -> 11.20 MB   (-1.37 MB, runtime)

Verified against a live backend: the glossary terms tab makes zero elk-related
requests and still renders, and /lineage fetches ELKUtil + vendor-elk on demand
and draws 83 nodes with no console errors.

Co-Authored-By: Claude <noreply@anthropic.com>
@siddhant1
siddhant1 force-pushed the long-animation-frame-block branch from 1def900 to 9a9143e Compare August 7, 2026 10:28
Copilot AI review requested due to automatic review settings August 7, 2026 10:28

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Destroying the TipTap instance on unmount exposed two setTimeout callbacks
in BlockEditor that touch the editor after the effect that scheduled them
has been cleaned up. They previously wrote into an orphaned-but-alive
editor; now they reach a torn-down ProseMirror view and throw
"Cannot read properties of null (reading 'updateOuterDeco')".

Clear the timers on cleanup and re-check isDestroyed inside the callback.
Clearing also drops a stale content write that could overwrite a newer one.

Cover the unmount path in useCustomEditor's own suite: MockEditor had no
destroy()/isDestroyed, so the new branch threw on every unmount and React
swallowed it while the suite still reported green.
Copilot AI review requested due to automatic review settings August 10, 2026 03:29

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
66.23% (78212/118081) 50.24% (47245/94020) 51.45% (14231/27659)

organize-imports-cli places the ElkLayoutUtils import after ExplorePureUtils.
Mechanical reformat only, from the same organize-imports + prettier sequence CI runs.
Copilot AI review requested due to automatic review settings August 10, 2026 03:47

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 10, 2026 03:49

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 51 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 51 warning(s) across 3 changed file(s).

Count Rule
36 react-hooks/exhaustive-deps
5 sonarjs/cyclomatic-complexity
4 sonarjs/no-duplicate-string
2 sonarjs/no-nested-functions
2 sonarjs/cognitive-complexity
1 jsx-a11y/no-static-element-interactions
1 sonarjs/no-nested-conditional
All findings
Location Rule Message
🟡 src/components/BlockEditor/BlockEditor.tsx:145:11 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 3 times.
🟡 src/components/BlockEditor/BlockEditor.tsx:243:8 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'isPopoverOpenRef'. Either include it or remove the dependency array.
🟡 src/components/BlockEditor/BlockEditor.tsx:277:7 jsx-a11y/no-static-element-interactions Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs
🟡 src/components/BlockEditor/hooks/useCustomEditor.ts:128:34 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/BlockEditor/hooks/useCustomEditor.ts:148:6 react-hooks/exhaustive-deps React Hook useEffect was passed a dependency list that is not an array literal. This means we can't statically verify whether you've passed the correct dependen
🟡 src/components/BlockEditor/hooks/useCustomEditor.ts:148:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'forceUpdate' and 'options'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:205:62 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 13 which is greater than 10 authorized.","cost":3,"secondaryLocations":[{"line":205,"column":61,"endLine":205,"endColum
🟡 src/context/LineageProvider/LineageProvider.tsx:336:9 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/context/LineageProvider/LineageProvider.tsx:470:6 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'isEditMode'. Either exclude it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:503:7 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 20 to the 15 allowed.
🟡 src/context/LineageProvider/LineageProvider.tsx:503:7 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 19 which is greater than 10 authorized.","cost":9,"secondaryLocations":[{"line":503,"column":6,"endLine":503,"endColumn
🟡 src/context/LineageProvider/LineageProvider.tsx:622:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setColumnsHavingLineage', 'setEdges', 'setIsRepositioning', and 'setNodes'. Either include them or remove the
🟡 src/context/LineageProvider/LineageProvider.tsx:681:13 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/context/LineageProvider/LineageProvider.tsx:682:23 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/context/LineageProvider/LineageProvider.tsx:690:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 't'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:749:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'isTourOpen', 'setEdges', 'setNodes', 't', and 'timeFilter'. Either include them or remove the dependency array
🟡 src/context/LineageProvider/LineageProvider.tsx:778:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'navigate', 'setActiveLayer', and 'setPlatformView'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:819:26 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/context/LineageProvider/LineageProvider.tsx:850:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'exportLineageData', 'showModal', and 't'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:862:75 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 20 which is greater than 10 authorized.","cost":10,"secondaryLocations":[{"line":862,"column":74,"endLine":862,"endColu
🟡 src/context/LineageProvider/LineageProvider.tsx:984:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'lineageData?.downstreamEdges', 'lineageData?.upstreamEdges', 't', and 'updateLineageData'. Either include them
🟡 src/context/LineageProvider/LineageProvider.tsx:1036:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setTracedColumns' and 'setTracedNodes'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1057:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'onPlatformViewChange' and 'setIsPlatformLineage'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1073:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'setSelectedEdge', 'setTracedColumns', and 'setTracedNodes'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1083:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setTracedColumns'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1241:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'edges', 'removeEdgesBySourceTarget', and 'removeNodeById'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1433:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setSelectedNode'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1456:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'selectLoadMoreNode', 'setActiveNode', 'setSelectedEdge', and 'setSelectedNode'. Either include them or remove
🟡 src/context/LineageProvider/LineageProvider.tsx:1466:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setActiveNode', 'setSelectedColumn', 'setSelectedNode', 'setTracedColumns', and 'setTracedNodes'. Either inclu
🟡 src/context/LineageProvider/LineageProvider.tsx:1478:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setActiveNode', 'setSelectedEdge', 'setSelectedNode', 'setTracedColumns', and 'setTracedNodes'. Either include
🟡 src/context/LineageProvider/LineageProvider.tsx:1526:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setSelectedEdge'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1529:33 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 14 which is greater than 10 authorized.","cost":4,"secondaryLocations":[{"line":1529,"column":32,"endLine":1529,"endCol
🟡 src/context/LineageProvider/LineageProvider.tsx:1626:31 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/context/LineageProvider/LineageProvider.tsx:1653:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'addTracedColumns', 'setColumnsHavingLineage', and 'setTracedNodes'. Either include them or remove the dependen
🟡 src/context/LineageProvider/LineageProvider.tsx:1672:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setSelectedEdge'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1752:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'handleModalCancel' and 'updateEdge'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1815:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setSelectedEdge' and 'updateEdge'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1878:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setActiveNode' and 'updateLineageData'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1883:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'redraw'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1885:47 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.
🟡 src/context/LineageProvider/LineageProvider.tsx:1885:47 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 21 which is greater than 10 authorized.","cost":11,"secondaryLocations":[{"line":1885,"column":46,"endLine":1885,"endCo
🟡 src/context/LineageProvider/LineageProvider.tsx:1938:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'fetchLineageData', 'fetchPlatformLineage', and 'timeFilter'. Either include them or remove the dependency arra
🟡 src/context/LineageProvider/LineageProvider.tsx:1965:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'setActiveLayer' and 'setLineageConfig'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2004:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'removeEdgeHandler' and 'removeNodeHandler'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2010:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'redraw'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2014:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'onPlatformViewUpdate'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2062:6 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'queryFilter'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2116:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'updateActiveLayer'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2122:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'fetchDataQualityLineage'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2164:6 react-hooks/exhaustive-deps React Hook useMemo has missing dependencies: 'platformView' and 't'. Either include them or remove the dependency array.

… and 1 more. Run make ui-checkstyle-changed locally for the full list.


Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

@sonarqubecloud

Copy link
Copy Markdown

@siddhant1
siddhant1 added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 2064dce Aug 10, 2026
85 of 87 checks passed
@siddhant1
siddhant1 deleted the long-animation-frame-block branch August 10, 2026 11:45
@github-actions

Copy link
Copy Markdown
Contributor

Changes have been cherry-picked to the 2.0 branch.

github-actions Bot pushed a commit that referenced this pull request Aug 10, 2026
…t engine on demand (#31157)

* fix(ui): destroy the TipTap editor on unmount so instances stop accumulating

`useCustomEditor` creates a ProseMirror `Editor` in an effect but its cleanup
only flipped an `isMounted` flag — `instance.destroy()` was never called
(grep finds it only in test files). The editor's view, plugin state,
contenteditable DOM and MutationObserver therefore outlive the component.

This compounds because the editor is mounted per table row: the description
column renders `RichTextEditorPreviewerNew` for every row, so a page of
descriptions is a page of editors. Measured live on release-2-0: 16 live
`.ProseMirror` instances for 16 rows on `/tags/General`, 7 for 7 on a glossary
terms table. Every pagination click, search keystroke or tab switch abandoned
that many editors for the rest of the session.

The `isDestroyed` guard mirrors `@tiptap/react`'s own `useEditor` cleanup
(node_modules/@tiptap/react/dist/index.js:1013 and :1039) — StrictMode
double-invokes effects, so the cleanup can run twice.

Co-Authored-By: Claude <noreply@anthropic.com>

* perf(ui): move ELK layout into its own module and load the engine on demand

`EntityLineageLayoutUtils` mixed two concerns: five pure geometry helpers
(`centerNodePosition`, `getNodesBoundsReactFlow`, `getViewportForBoundsReactFlow`,
...) and two ELK-backed layout functions. Because `ELKUtil` was imported at
module scope, importing any of the pure helpers dragged elkjs — 1.37MB built.

`LineageControlButtons` imports only `centerNodePosition`, and `LineageProvider`
is statically imported by eight lineage components. That was enough to land
elkjs in the shared chunk which 476 route chunks statically import, so every
authenticated page downloaded and parsed the lineage layout engine. Glossary
pages paid for it while rendering with G6/antv-dagre, never calling ELK at all.

`getELKLayoutedElements` and `positionNodesUsingElk` now live in
`utils/Lineage/Layout/ElkLayoutUtils.ts`, alongside a named `loadElkLayout()`
lazy boundary. `EntityLineageLayoutUtils` no longer references ELK in any form,
so the pure helpers cannot re-acquire the dependency by accident.

A plain file split would not have been enough on its own: `LineageProvider` is
the sole caller of the ELK functions and is itself reachable from the shared
cluster, so the engine needs the dynamic boundary regardless. Making it a named
function in an ELK-specific module keeps that explicit rather than burying an
`await import()` in the middle of layout logic.

Measured on a production build:

  ELKUtil chunk importers   1 static -> 0 static, 1 dynamic
  glossary terms tab        12.57 MB -> 11.20 MB   (-1.37 MB, runtime)

Verified against a live backend: the glossary terms tab makes zero elk-related
requests and still renders, and /lineage fetches ELKUtil + vendor-elk on demand
and draws 83 nodes with no console errors.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(ui): guard deferred BlockEditor timers against the destroyed editor

Destroying the TipTap instance on unmount exposed two setTimeout callbacks
in BlockEditor that touch the editor after the effect that scheduled them
has been cleaned up. They previously wrote into an orphaned-but-alive
editor; now they reach a torn-down ProseMirror view and throw
"Cannot read properties of null (reading 'updateOuterDeco')".

Clear the timers on cleanup and re-check isDestroyed inside the callback.
Clearing also drops a stale content write that could overwrite a newer one.

Cover the unmount path in useCustomEditor's own suite: MockEditor had no
destroy()/isDestroyed, so the new branch threw on every unmount and React
swallowed it while the suite still reported green.

* Fix UI checkstyle

organize-imports-cli places the ElkLayoutUtils import after ExplorePureUtils.
Mechanical reformat only, from the same organize-imports + prettier sequence CI runs.

---------

Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit 2064dce)
@github-actions

Copy link
Copy Markdown
Contributor

Failed to cherry-pick changes to the 1.13 branch.
Please cherry-pick the changes manually.
You can find more details here.

@gitar-bot

gitar-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Releases TipTap editor instances on unmount to prevent DOM accumulation and moves the ELK layout engine to an on-demand dynamic import, reducing bundle size by 1.37MB across authenticated routes. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release TipTap editor instances on unmount so long sessions stay responsive

3 participants