Fixes #30898: enforce frontend performance patterns

[shah-harshit]

Fixes #30898

Describe your changes:

This PR converts the recent frontend performance practices into explicit ESLint policy. It adds three repository-specific, reporting-only rules, clears every existing violation, and enables all three at error severity so regressions block local lint and UI Checkstyle immediately.

Enforcement levels

Level	Meaning in this PR	Result
Mandatory / blocking	Rule has zero measured backlog and is configured as error.	Any finding fails ESLint and must be fixed before merge.
Advisory / non-blocking	Rule has an existing measured backlog and remains configured as warn.	Finding is visible locally and in CI output but does not fail the PR by itself.
Disabled	Rule is unsafe or too noisy for the current workflow.	No finding is produced until the prerequisite cleanup/configuration is completed.

Severity is based on measured repository backlog rather than preference. A rule is promoted from warn to error only after its backlog reaches zero and a full-tree scan confirms that it can block without failing unrelated files.

Mandatory performance rules added by this PR

1. openmetadata-performance/no-eager-page-imports

• Severity: error — mandatory and blocking.
• Scope: src/components/AppRouter/**/*.{ts,tsx} only.
• Purpose: preserve route-level code splitting and prevent page bundles from moving into the initial router bundle.
• Rejected: any static runtime import whose source path contains pages/.
• Allowed:
  • import type ... declarations;
  • named imports where every imported specifier is type-only;
  • dynamic imports used through React lazy();
  • imports outside AppRouter modules.
• Expected remediation: load the page with lazy() and expose it through withPageSuspenseFallback() or another approved route fallback path.

Example rejected pattern:

import MyPage from '../../pages/MyPage/MyPage';

Example accepted pattern:

const MyPage = withPageSuspenseFallback(
  lazy(() => import('../../pages/MyPage/MyPage'))
);

2. openmetadata-performance/require-suspense-fallback

• Severity: error — mandatory and blocking.
• Scope: UI JavaScript and TypeScript source files covered by the main ESLint source configuration.
• Purpose: ensure every React-lazy component has a loading boundary and cannot suspend without fallback UI.
• Recognized lazy APIs:
  • named or aliased lazy imports from react;
  • React.lazy through a React default or namespace import.
• Accepted protection:
  • the lazy() call is passed directly to withSuspenseFallback or withPageSuspenseFallback imported from a module ending in /withSuspenseFallback;
  • a previously declared lazy binding is subsequently passed to an approved helper;
  • the lazy binding, or a local variable/registry derived from it, is rendered or passed beneath a real React <Suspense fallback={...}> boundary.
• Rejected:
  • an unwrapped lazy component;
  • <Suspense> without an explicit fallback prop;
  • a same-named local component that shadows the actual lazy binding;
  • an unrelated Suspense boundary elsewhere in the module;
  • a lazy component that is also rendered outside its protected boundary.
• Analysis details: associations are scope-aware and follow local variable dependencies, including typed aliases, component maps, and memoized field/template registries. Protection is attached to the resolved binding rather than the identifier text or a module-wide boolean.

3. openmetadata-performance/no-unbounded-module-cache

• Severity: error — mandatory and blocking.
• Scope: UI JavaScript and TypeScript source files covered by the main ESLint source configuration.
• Purpose: prevent module-lifetime caches from growing with catalog data until the browser tab runs out of memory.
• Candidates checked: module-level or directly exported new Map() / new Set() bindings whose identifier contains cache or memo, case-insensitively.
• Required bounded pattern:
  • compare the same binding's .size using >, >=, or the equivalent reversed < / <= form;
  • compare against either a numeric literal or an uppercase constant such as MAX_CACHE_SIZE;
  • call .delete(...) or .clear() on that same resolved binding inside the matching if branch or while body.
• Rejected:
  • a cache with no size guard;
  • a size guard used only for logging or metrics;
  • normal invalidation elsewhere in the file that is not executed by the overflow guard;
  • eviction of a shadowed same-name local cache;
  • a lowercase dynamic limit such as maxEntries, which does not establish an explicit repository constant.
• Intentionally ignored: function-local Map/Set instances and module-level collections without cache-like names, because they are not module-lifetime cache candidates for this heuristic.

Advisory performance rules — enabled but not mandatory yet

These existing React rules remain at warn because the repository still has measured violations. This PR documents their current backlog but does not promote them to blocking:

Rule	Practice enforced	Current measured backlog
react-hooks/exhaustive-deps	Keep hook dependency arrays complete so effects and memoization do not use stale values.	1,693 findings across 596 files
react/no-array-index-key	Use stable entity keys instead of array positions to avoid incorrect reconciliation and remounts.	93 findings across 59 files
react/jsx-no-constructed-context-values	Avoid constructing a new context value on every provider render.	8 findings across 7 files
react/no-unstable-nested-components	Avoid defining component types during render, which causes remounts and state loss.	25 findings across 23 files

These warnings are visible in editors and CI output. They become mandatory only after their backlog is fixed and re-measured at zero.

Advisory import architecture and request rules

This PR also adds ten reporting-only OpenMetadata rules at warn. They are visible on changed files
but do not fail UI Checkstyle while the measured backlog is reduced. None of them autofix source.

Rule	Enforcement and intent	Measured baseline
openmetadata-imports/no-impure-pure-utils	Advisory. Pure utilities cannot contain JSX or depend on React, UI/state layers, pages, hooks, or REST clients.	62 findings / 23 files
openmetadata-imports/no-lower-layer-page-imports	Advisory. Pages remain route-level composition leaves; AppRouter is the explicit owner exception.	291 / 271
openmetadata-imports/no-cross-page-imports	Advisory. Page features cannot statically depend on other page features.	43 / 32
openmetadata-imports/no-rest-ui-imports	Advisory. REST clients cannot depend upward on components, pages, hooks, context, or stores.	55 / 37
openmetadata-imports/no-hook-ui-imports	Advisory. Hooks cannot depend on components or pages.	10 / 6
openmetadata-imports/no-circular-imports	Advisory. Reports runtime import/re-export cycles; type-only dependencies are ignored.	295 / 164
openmetadata-imports/no-internal-barrel-imports	Advisory. Reports runtime imports through app-internal index barrels; type-only imports are allowed.	143 / 134
openmetadata-imports/no-lodash-default-import	Advisory. Requires named or direct-member Lodash imports instead of the package default/namespace object.	1 / 1
openmetadata-imports/no-api-calls-in-iteration	Advisory. Identifies potential N-request patterns in loops and dynamic iteration callbacks.	28 / 21
openmetadata-imports/review-sequential-api-calls	Review-only advisory. Identifies directly awaited REST calls on compatible execution paths; mutually exclusive if/else, switch, and try/catch branches are excluded.	84 / 55

Where warnings appear in CI

For pull requests, the ui-checkstyle workflow posts or updates a sticky GitHub Actions comment
titled UI Checkstyle passed — lint findings in changed files. It groups findings by rule and
includes the changed file, line, column, and message. The same warnings appear in
Actions → UI Checkstyle → checkstyle → ESLint + Prettier + Organise Imports (src). Warnings do
not fail the check; ESLint errors or formatter-generated diffs still block it.

The deterministic rules can move to error after their full-tree baseline reaches zero. The
sequential-request review rule must be re-evaluated separately because static analysis cannot prove
request independence.

Additional validation

• Added RuleTester coverage for all ten warning rules, including scope shadowing, type-only edges,
  live cycle-cache invalidation, and runtime barrel detection.
• Ran all 82 custom ESLint rule tests successfully.
• Ran the full UI source lint inventory: 0 errors and 9,997 warnings across 2,226 files.
• Ran make ui-checkstyle-changed; warning output remained non-blocking and the check completed
  successfully.
• Simulated the PR warning reporter and verified rule grouping plus file/line details.

Rule intentionally not enabled

• react/jsx-no-useless-fragment remains disabled. The rule auto-fixes source files, while UI Checkstyle runs ESLint with --fix and then rejects any generated diff. It can be enabled only after a dedicated repository-wide autofix commit removes the backlog.

Autofix policy

The three new OpenMetadata performance rules intentionally have no autofix. Choosing the correct loading boundary, eviction policy, cache limit, or eager-versus-lazy route dependency requires runtime context. ESLint reports the violation, but the author must make the architectural decision.

Type of change:

• Improvement

High-level design:

• Add a local flat-config ESLint plugin with reporting-only AST rules and RuleTester coverage.
• Apply lazy-boundary and bounded-cache enforcement across UI source files.
• Scope page-import enforcement to AppRouter modules, where page-level code splitting is required.
• Resolve imported and local variables through ESLint scope information so shadowed names cannot satisfy a rule.
• Follow local dependency paths so lazy components passed through aliases, maps, memoized registries, RJSF fields, templates, or widgets remain correctly associated with their Suspense boundary.
• Require cache eviction to occur causally inside the matching overflow guard.
• Preserve route-level and tab-level loading behavior through the existing Suspense helpers.
• Add explicit cache limits and eviction for edge styles, graph textures, ontology colors, task schemas, and form-field documentation.
• Document the two-tier severity policy and zero-backlog promotion path.

Off-the-shelf rules do not understand the repository fallback helpers, router path convention, variable/registry flows, or cache naming and eviction policy. Warning-only rollout was not needed for the three custom rules because their complete existing backlog was cleared before enabling them.

No migration or backward-compatibility action is required.

Tests:

Use cases covered

• Static AppRouter page imports fail while type-only imports remain valid.
• Classification and glossary page chunks remain deferred and render through route fallbacks.
• Lazy bindings are associated only with their actual Suspense boundary, including aliases, maps, and memoized registries.
• Unrelated boundaries and shadowed same-name bindings do not create false exemptions.
• Custom dashboard widgets and entity-version components render through scoped Suspense boundaries.
• Module-level caches remain bounded and evict entries from the matching overflow guard.
• Unrelated invalidation, logging-only guards, and shadowed local caches do not satisfy cache enforcement.
• Valid and invalid examples are covered for all three custom rules.

Unit tests

• Added 42 RuleTester cases for the three custom ESLint rules.
• Ran 64 focused Jest tests across routers, fallbacks, widgets, entity versions, edge styles, and task schemas.
• Ran 65 Playwright CI-planning tests after excluding delegated RDF specs from the main planner.
• Ran the full UI source tree through ESLint with zero errors from the three new rules.
• Verified the PR's targeted Playwright selector and generated an 11-shard plan.

Backend integration tests

• Not applicable; no backend API changes.

Ingestion integration tests

• Not applicable; no ingestion changes.

Playwright (UI) tests

• No browser workflow behavior changed; targeted Playwright selection and shard planning were validated.

Manual testing performed

Not applicable; this change is covered by AST rule tests, focused UI unit tests, full-tree ESLint, UI Checkstyle, and Playwright planner validation.

UI screen recording / screenshots:

Not applicable; loading indicators and rendered UI remain unchanged.

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes issue-number: short explanation.
• My PR is linked to a GitHub issue.
• I have commented on code where non-obvious behavior requires context.
• JSON Schema migration is not applicable.
• UI screenshots are not applicable because there is no visual change.
• I have added tests and listed them above.
• I have updated the frontend performance and UI quality-gate documentation.

[github-actions]

❌ PR checklist incomplete

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

• No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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]

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]

✅ Playwright Results — workflow succeeded

Validated commit 5935442798cf7ef08b932cf92f2f81f86d2951aa in Playwright run 31088959974, attempt 1.

✅ 680 passed · ❌ 0 failed · 🟡 1 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 51s

⏱️ Max setup 3m 10s · max shard execution 17m 49s · max shard-job elapsed before upload 21m 15s · reporting 5s

🌐 191.08 requests/attempt · 2.37 app boots/UI scenario · 6.64% common-shard skew

Optimization targets still in progress:

• Application boot ratio was 2.37 per UI scenario (1811 boots / 764 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard chromium-01	121	0	0	0	0	0
✅ Shard chromium-02	130	0	0	0	0	0
🟡 Shard chromium-03	118	0	1	0	0	0
✅ Shard chromium-04	160	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

🟡 1 flaky test(s) (passed on retry)

• Features/OntologyExplorerCardinality.spec.ts › CUSTOM relation type with sourceMax=1 and no targetMax should produce "1" → "M" (shard chromium-03, 1 retry)

📦 Download artifacts

How to debug locally

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

[github-actions]

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — ESLint report could not be read — see the job log.

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.

No machine-readable report was produced.

---

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

make ui-checkstyle-changed

[gitar-bot]

Confirmed both fixes in the latest push.

• no-unbounded-module-cache: hasBoundedEviction (openmetadata-performance.mjs:462) now requires the delete/clear call to be inside the if/while body whose test is a matching size guard for the same resolved variable, via isInside. Regression cases for unrelated invalidation outside the guard and a shadowed local cache are both covered in the test file (lines 348-374).
• require-suspense-fallback: JSX usages are now tied to their owning variable through variableDependencies/getFallbackProtectedBindings, and isRenderedWithFallback checks that every JSX usage of that specific binding is under a real fallback-bearing Suspense. The shadowed-component and fake-Suspense-binding cases are covered (lines 252-271).

No further action needed here.

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	66% (77647/117639)	49.96% (46839/93744)	51.16% (14089/27535)

[gitar-bot]

Code Review ✅ Approved 5 resolved / 5 findings

Enforces frontend performance patterns by adding blocking ESLint rules for route code-splitting, suspense fallbacks, and bounded caches, while clearing existing violations. Resolved issues include scope-aware suspense boundary checks, causal cache eviction validation, member-expression rendering support, and duplicate license headers.

✅ 5 resolved

✅ Quality: require-suspense-fallback exempts whole file on one Suspense

📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-performance.mjs:58 📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-performance.mjs:151-165 📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-performance.mjs:182-190
hasExplicitSuspenseFallback is a single module-wide boolean: as soon as any <Suspense fallback=...> appears anywhere in a file, the Program:exit handler skips reporting ALL unwrapped lazy() calls in that file. A file where component A is correctly rendered under a Suspense boundary but component B is rendered with no boundary would pass this blocking rule, yet B would throw "a component suspended... no fallback UI was specified" at runtime. Since the rule is enforced at error to guarantee every lazy component has a boundary, consider tracking the association between each lazy binding and an actual enclosing Suspense (or only exempting lazy calls that are themselves wrapped) rather than a global flag.

✅ Quality: Cache size-guard and eviction checks are not causally linked

📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-performance.mjs:250-264 📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-performance.mjs:287-296
hasSizeGuard and hasEviction scan the entire file independently, matching only on the cache's identifier name. A cache that has a .delete(key) used for normal invalidation plus an unrelated size > N comparison used only for logging/metrics passes the rule even though it never evicts on overflow. The rule therefore accepts genuinely unbounded caches. This is only a false-negative in a heuristic rule (not a runtime bug), but consider verifying the eviction occurs inside the overflow branch, e.g. requiring the delete/clear call to be within the consequent of the size-guard comparison.

✅ Edge Case: Suspense rule ignores JSX member-expression rendering

📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-performance.mjs:232-246 📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-performance.mjs:307-320
trackJsxUsage only records usages when node.name is a JSXIdentifier, so a lazy component stored in an object/array and rendered via a member expression (e.g. <route.component/> or <obj.Page/>) beneath a Suspense-with-fallback is never marked protected, causing isRenderedWithFallback to return false and a false 'missingSuspenseFallback' error. No current source triggers this (verified), but since the rule runs at error severity across the UI tree it can block CI for a legitimate future pattern. Consider also handling JSXMemberExpression names in trackJsxUsage/isInsideSuspenseWithFallback by resolving the root object identifier's variable.

✅ Quality: review-sequential-api-calls flags mutually exclusive awaits

📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-imports.mjs:564-578
The rule collects every awaited API call in a function and reports all but the first (awaits.slice(1)), ignoring control flow. Two awaits in an if/else, switch, or try/catch are never both executed at runtime yet both get grouped under the same owning function, producing false 'sequential' findings. Since the rule is warn-only and explicitly framed as a review prompt this is low impact, but it will generate noise; consider only pairing awaits that share the same execution path (e.g. sequential statements in one block) before reporting.

✅ Quality: Duplicate Apache license header in new rule file

📄 openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-imports.mjs:1-15
The new file begins with two identical Apache 2.0 / Collate copyright blocks (lines 1-12 and 13-26), clearly a copy-paste artifact. Remove one block to match the single-header convention used elsewhere in the repo.

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 👍 / 👎 | Gitar | Powered by Gitar — free for open source}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud