Skip to content

Quality pass: library packaging, Job Monitor performance, state correctness, test coverage #494

Description

@aldbr

Context

While working on the data-table performance refactor (#483), a full review of
diracx-web-components and the app shells surfaced problems in four areas. The
resulting work is too large to review as a single PR (+5000 lines), so it is
being split into a stack of focused PRs, all tracked by this issue.

What this covers

1. npm packaging of the published library

@dirac-grid/diracx-web-components currently only works inside this monorepo:

  • No peerDependencies: react, react-dom, @mui/*, @emotion/*,
    @axa-fr/react-oidc are hard dependencies — consumers risk duplicate
    React/emotion instances ("Invalid hook call", broken styles) and a duplicated
    oidc module whose auth state silently breaks.
  • next/image is imported in two components while Next.js is not declared as a
    dependency at all — the library is unusable outside Next.js apps.
  • package.json#exports has no "." entry, so the root import advertised by
    main/module fails; no sideEffects: false, so tree-shaking and the lazy
    @mui/x-charts split are ineffective.

2. Job Monitor performance

  • Every page/sort/filter change unmounted the table into a skeleton
    (no keepPreviousData); it now keeps stale rows visible with a progress bar.
  • overscan: 50 and no memoized row component meant ~125 MUI rows
    (~1400 cells) re-rendered on every scroll frame and every state change.
  • One monolithic context value: a single row-checkbox click re-rendered the
    search bar, the pie chart, and every status cell.
  • Filter value suggestions fired uncached, unabortable jobs/summary group-by
    queries — several per restored filter, before any user interaction.
  • The jobs search fetched all 18 columns while 8 are hidden by default
    (no parameters projection).
  • Changing rows-per-page could serve minutes-old cached data
    (revalidateIfStale: false combined with a 60 s deduping interval).

3. State correctness and UX

  • React state mutated in place in the dashboard import/creation flows and the
    search bar — latent StrictMode/concurrent-rendering bugs; one dialog opened
    only as a side effect of an unrelated re-render.
  • Index keys on drag-reorderable lists (dashboard groups, filter equations).
  • API responses and sessionStorage content trusted via blind casts — e.g. an
    empty job-history response crashed instead of showing the error snackbar.
  • Token classification in the search bar raced the debounced suggestions
    effect: typing ID, =, 1 quickly silently dropped tokens.
  • Assorted: sessionStorage writes during render in ThemeProvider, hardcoded
    light-mode focus colors in dark mode, inconsistent error surfaces, missing
    keyboard access and aria labels on the pie-chart legend and column toggles.

4. Test coverage and tooling

  • Unit tests existed only for components with Storybook stories; the search-bar
    reducer, filter sync, and persistence hooks had none. The suite grew from
    60 tests / 7 suites to 93 tests / 11 suites.
  • Two suites had vacuous or race-prone assertions; structuredClone was
    polyfilled with a lossy JSON round-trip.
  • jsx-a11y linting was missing in the components package (where all the
    interactive UI lives); the app ESLint configs re-enabled formatting rules
    that conflict with Prettier.
  • Hygiene: .nvmrc + engines, headless Cypress script, coverage script.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions