From 9551d7a23d9376e89418d198f05f55baccf520cd Mon Sep 17 00:00:00 2001 From: Michelle Jameson <241148084+chellej@users.noreply.github.com> Date: Wed, 30 Sep 2026 03:16:27 -0500 Subject: [PATCH 1/5] fix(build): make index.d.ts and the runtime bundle export the same names src/custom/ had two barrels, index.ts and index.tsx, exporting different lists. `import './custom'` resolves by extension order and the two builds disagree on it: esbuild (runtime) picks .tsx, TypeScript (declarations) picks .ts. So 0.22.x declared HelperTextPopover and RenderMarkdownTooltip without shipping them (typecheck green, runtime crash) and shipped 130 exports it never declared (L5DeleteIcon, L5EditIcon, TooltipIcon, WorkspaceCard, ...). This was long misread as a rollup-plugin-dts "nested-barrel drop". - Merge the barrels into custom/index.tsx and delete index.ts; drop the identical duplicate icons/Chain/index.tsx. - Make the newly-declared code pass the dts type-check: type mui-datatables imports through @types/mui-datatables (a declared dependency) with a runtime-only shim for the untyped fork, and fix the strict-mode errors the previously unreached files carried. - Stop naming bundled devDependencies (react-error-boundary, notistack) in the public types; export local ErrorFallbackProps and NotificationHandlerOptions, checked against the originals at build. - Add declarationRuntimeExportParity.test.ts: rejects any .ts/.tsx pair in src/, and compares dist/index.d.ts value exports with dist/index.js (via cjs-module-lexer, as Node reads it) and dist/index.mjs in both directions. Fails on the previous build, passes on this one. - Teach the type-surface guard that @types/foo supplies `foo`. - Correct AGENTS.md and the src/index.tsx comments that blamed rollup-plugin-dts. Signed-off-by: Michelle Jameson <241148084+chellej@users.noreply.github.com> --- AGENTS.md | 51 ++--- package-lock.json | 7 +- package.json | 1 + src/__testing__/RJSFFormWrapper.test.tsx | 17 +- .../declarationRuntimeExportParity.test.ts | 201 ++++++++++++++++++ .../publishedTypeSurfaceDependencies.test.ts | 42 +++- .../CatalogDesignTable/CatalogDesignTable.tsx | 10 +- .../DesignTableColumnConfig.tsx | 2 +- .../CatalogDesignTable/columnConfig.tsx | 2 +- src/custom/CustomCatalog/Helper.ts | 14 +- .../CustomCatalog/sanitizeCatalogImageUrl.ts | 7 - .../CustomColumnVisibilityControl.tsx | 2 +- src/custom/ErrorBoundary/ErrorBoundary.tsx | 26 ++- src/custom/ErrorBoundary/index.tsx | 3 +- src/custom/FlipCard/FlipCard.tsx | 2 +- src/custom/Helpers/Notification/index.tsx | 4 +- .../Notification/notification-handler.ts | 19 +- .../ResourceDetailFormatters/Formatter.tsx | 3 +- src/custom/ResponsiveDataTable.tsx | 9 +- src/custom/TableActions.tsx | 8 - src/custom/TeamTable/TeamTable.tsx | 2 +- .../TeamTable/TeamTableConfiguration.tsx | 2 +- .../TransferList/TransferList.tsx | 2 +- .../UserSearchField/UserSearchField.tsx | 2 +- src/custom/UsersTable/UsersTable.tsx | 13 +- src/custom/Workspaces/EnvironmentTable.tsx | 11 +- src/custom/Workspaces/WorkspaceViewsTable.tsx | 11 +- src/custom/index.ts | 10 - src/custom/index.tsx | 13 +- src/icons/Chain/index.tsx | 1 - src/index.tsx | 86 ++------ src/theme/components/table.modifier.ts | 6 +- src/types/sistent-mui-datatables.d.ts | 15 ++ 33 files changed, 418 insertions(+), 186 deletions(-) create mode 100644 src/__testing__/declarationRuntimeExportParity.test.ts delete mode 100644 src/custom/index.ts delete mode 100644 src/icons/Chain/index.tsx create mode 100644 src/types/sistent-mui-datatables.d.ts diff --git a/AGENTS.md b/AGENTS.md index 217a5f976..57a2028e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -203,37 +203,26 @@ JS - see `useIsNavigationItemPermitted` and its callers in which withhold both `onClick` and the expand toggle so an unpermitted section cannot navigate or open its children. -## New public exports need an explicit root re-export - -`rollup-plugin-dts` (used by tsup for the declaration bundle) silently drops symbols that reach -the root barrel only through a nested `export * from './custom'` (or `./base`, etc.) - the runtime -export in `dist/index.*js` survives, but the declaration is missing from `dist/index.d.ts`, so -`import { Foo, type FooProps } from '@sistent/sistent'` fails type-checking downstream. When you add -a new public component or type in a `src//` subtree, also add an explicit -`export { Foo, type FooProps } from './/Foo';` to `src/index.tsx` (see the documented block -of examples there, e.g. `FeedbackButton`, `NavigationItem`). Verify by building and grepping -`dist/index.d.ts` for the symbol - a green `jest`/lint run will not catch this. - -The explicit block is a stopgap, not the fix. Measure the real gap before assuming a symbol is -covered - it is large, and every uncovered symbol is one a consumer must shim locally: - -```bash -npm run build -node -e 'const f=require("fs"),names=t=>{const s=new Set(); -for(const b of t.matchAll(/export\s*\{([^{}]*)\}\s*;?/g)) - b[1].split(",").map(x=>x.trim()).filter(Boolean) - .forEach(x=>s.add(x.replace(/^type\s+/,"").split(/\s+as\s+/).pop().trim())); -return s;}; -const rt=names(f.readFileSync("dist/index.mjs","utf8")); -const dt=names(f.readFileSync("dist/index.d.ts","utf8")); -console.log([...rt].filter(n=>/^[A-Za-z_$][\w$]*$/.test(n)&&!dt.has(n)).sort().join("\n"))' -``` - -As of this change that reports 130 of 729 runtime exports absent from the declaration bundle - -`WorkspaceCard`, `TeamTable`, `UsersTable`, `CustomImage`, `ErrorBoundary` and most of -`src/custom/` among them. Adding 130 lines is not the answer; the durable fix is in how the -declaration bundle is produced. Until then, prefer extending this list over leaving a symbol -uncovered, and do not read its absence as "that component is intentionally private". +## Declarations and runtime must export the same names + +The runtime bundle (esbuild) and the declaration bundle (TypeScript, via tsup's dts step) resolve +`import './foo'` by extension in opposite orders: esbuild tries `.tsx` first, TypeScript `.ts`. So a +`foo.ts` beside a `foo.tsx` makes each build export a different list, silently. That is how +`src/custom/` once had two barrels and 0.22.x shipped 130 undeclared exports (`WorkspaceCard`, +`TeamTable`, ...) and declared two it never shipped (`HelperTextPopover`, `RenderMarkdownTooltip`) - +long misread as a `rollup-plugin-dts` "nested-barrel drop", which is why `src/index.tsx` still +carries explicit re-exports that are no longer load-bearing. + +[`src/__testing__/declarationRuntimeExportParity.test.ts`](src/__testing__/declarationRuntimeExportParity.test.ts) +is the guard: it rejects any `.ts`/`.tsx` pair in `src/`, and compares the built `dist/index.d.ts` +value exports with `dist/index.js` and `dist/index.mjs` in both directions (skips without a build, +fails in CI without one - same contract as the type-surface guard above). + +The dts step type-checks everything reachable from `src/index.tsx`, so code newly exported must +type-check under `strict`, and a package it imports must have types. `@sistent/mui-datatables` +ships none: import its types with `import type { ... } from 'mui-datatables'` (typed by the +`@types/mui-datatables` dependency) - the ambient shim in `src/types/sistent-mui-datatables.d.ts` +covers only the runtime default import and is not published. ## Repo state that looks broken but is pre-existing diff --git a/package-lock.json b/package-lock.json index c3f5af590..43285cb3d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -50,6 +50,7 @@ "@types/react-redux": "^7.1.34", "@typescript-eslint/eslint-plugin": "^8.59.1", "@typescript-eslint/parser": "^8.59.1", + "cjs-module-lexer": "^2.2.1", "commitizen": "^4.3.1", "cz-conventional-changelog": "^3.3.0", "date-fns": "^4.4.0", @@ -6645,9 +6646,9 @@ } }, "node_modules/cjs-module-lexer": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/cjs-module-lexer/-/cjs-module-lexer-2.2.0.tgz", - "integrity": "sha512-4bHTS2YuzUvtoLjdy+98ykbNB5jS0+07EvFNXerqZQJ89F7DI6ET7OQo/HJuW6K0aVsKA9hj9/RVb2kQVOrPDQ==", + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/cjs-module-lexer/-/cjs-module-lexer-2.2.1.tgz", + "integrity": "sha512-Ca8swihM+/4yKecYHY52kgJd300hi2lADU/a1RxNTRe+RJ9jvqQlESpbz9DnG9mowez8qwXHB8qYdIUw9e+F5Q==", "dev": true, "license": "MIT" }, diff --git a/package.json b/package.json index bf3a72522..a45f1c878 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "@types/react-redux": "^7.1.34", "@typescript-eslint/eslint-plugin": "^8.59.1", "@typescript-eslint/parser": "^8.59.1", + "cjs-module-lexer": "^2.2.1", "commitizen": "^4.3.1", "cz-conventional-changelog": "^3.3.0", "date-fns": "^4.4.0", diff --git a/src/__testing__/RJSFFormWrapper.test.tsx b/src/__testing__/RJSFFormWrapper.test.tsx index c106c6574..11f25ba52 100644 --- a/src/__testing__/RJSFFormWrapper.test.tsx +++ b/src/__testing__/RJSFFormWrapper.test.tsx @@ -32,16 +32,9 @@ describe('RJSFFormWrapper (sistent#1533)', () => { ).toBe('RJSFFormWrapper'); }); - it.each([ - ['index.ts', 'src/custom/index.ts'], - ['index.tsx', 'src/custom/index.tsx'] - ])( - 'src/custom/%s re-exports the RJSFFormWrapper module', - (_label, relPath) => { - const full = path.resolve(__dirname, '..', '..', relPath); - expect(fs.existsSync(full)).toBe(true); - const source = fs.readFileSync(full, 'utf8'); - expect(source).toMatch(/export\s+\*\s+from\s+['"]\.\/RJSFFormWrapper['"]/); - } - ); + it('src/custom/index.tsx re-exports the RJSFFormWrapper module', () => { + const full = path.resolve(__dirname, '..', '..', 'src/custom/index.tsx'); + const source = fs.readFileSync(full, 'utf8'); + expect(source).toMatch(/export\s+\*\s+from\s+['"]\.\/RJSFFormWrapper['"]/); + }); }); diff --git a/src/__testing__/declarationRuntimeExportParity.test.ts b/src/__testing__/declarationRuntimeExportParity.test.ts new file mode 100644 index 000000000..6825cb939 --- /dev/null +++ b/src/__testing__/declarationRuntimeExportParity.test.ts @@ -0,0 +1,201 @@ +/** + * @jest-environment node + */ +/** + * `dist/index.d.ts` and the runtime bundles must export the same values. + * + * Consumers treat the declarations as the answer to "does sistent export this", + * so the two drifting apart fails in the worst available order. A name declared + * but not shipped type-checks and then throws at runtime (`HelperTextPopover` + * and `RenderMarkdownTooltip` did, in 0.22.x); a name shipped but not declared + * fails to type-check although it works, and pushes every consumer into a local + * `declare module` shim (130 of them did, `WorkspaceCard` and most of + * `src/custom/` among them). + * + * Both came from one cause: `src/custom/` had an `index.ts` *and* an + * `index.tsx`, each a different barrel. `import './custom'` resolves by extension + * order, and the two builds disagree on it - esbuild (the runtime bundle) tries + * `.tsx` first, TypeScript (the declaration bundle) tries `.ts` first - so each + * build silently exported a different list. `the source tree` below rules that + * shape out before it can reach a build; the `dist/` checks catch any other way + * the two builds come to disagree. + * + * Runtime exports are read the way consumers read them: `cjs-module-lexer` is + * what Node itself uses to expose a CommonJS module's named exports to `import`, + * and `package.json` points both `main` and `module` at the CommonJS bundle. + * Declared exports are read with the TypeScript checker, because the bundle + * re-exports types and values in the same `export { ... }` lists and only the + * checker can tell them apart - a type-only export correctly has no runtime + * counterpart. + */ +import { parse as parseCommonJs } from 'cjs-module-lexer'; +import fs from 'fs'; +import path from 'path'; +import ts from 'typescript'; + +const ROOT = path.resolve(__dirname, '..', '..'); +const SRC = path.join(ROOT, 'src'); +const DIST = path.join(ROOT, 'dist'); +const DTS = path.join(DIST, 'index.d.ts'); +const CJS = path.join(DIST, 'index.js'); +const ESM = path.join(DIST, 'index.mjs'); + +/** Every source file, recursively, as a path relative to `src/`. */ +const sourceFiles = (dir: string): string[] => + fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return sourceFiles(full); + return /\.(ts|tsx)$/.test(entry.name) && !entry.name.endsWith('.d.ts') + ? [path.relative(SRC, full)] + : []; + }); + +/** + * Module paths that more than one file answers to - `foo.ts` next to + * `foo.tsx`, which is where the two builds' resolution orders diverge. + */ +const ambiguousModulesIn = (files: string[]): string[] => { + const byModule = new Map(); + for (const file of files) { + const specifier = file.replace(/\.(ts|tsx)$/, ''); + byModule.set(specifier, [...(byModule.get(specifier) ?? []), file]); + } + return [...byModule.entries()] + .filter(([, candidates]) => candidates.length > 1) + .map(([specifier, candidates]) => `${specifier}: ${candidates.sort().join(', ')}`) + .sort(); +}; + +/** Exported names of the declaration bundle, split by whether they carry a value. */ +const declaredExportsOf = (dtsFile: string): { values: Set; all: Set } => { + const program = ts.createProgram([dtsFile], { + noEmit: true, + skipLibCheck: true, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + jsx: ts.JsxEmit.ReactJSX + }); + const checker = program.getTypeChecker(); + const source = program.getSourceFile(dtsFile); + const moduleSymbol = source && checker.getSymbolAtLocation(source); + if (!moduleSymbol) throw new Error(`TypeScript could not load ${dtsFile} as a module`); + + const values = new Set(); + const all = new Set(); + for (const exported of checker.getExportsOfModule(moduleSymbol)) { + all.add(exported.name); + const target = + exported.flags & ts.SymbolFlags.Alias ? checker.getAliasedSymbol(exported) : exported; + // An alias that does not resolve (its package is missing) would read as + // type-only and quietly drop out of the comparison, so it has to fail here. + if (!target.declarations?.length) { + throw new Error(`${exported.name} in ${dtsFile} does not resolve to a declaration`); + } + if (target.flags & ts.SymbolFlags.Value) values.add(exported.name); + } + return { values, all }; +}; + +const commonJsExportsOf = (file: string): Set => { + const { exports, reexports } = parseCommonJs(fs.readFileSync(file, 'utf8')); + // A `module.exports = require(...)` re-export hides its names from the lexer, + // and the comparison would then under-report the runtime surface. + if (reexports.length) throw new Error(`${file} re-exports ${reexports.join(', ')} opaquely`); + return new Set(exports.filter((name) => name !== '__esModule')); +}; + +/** + * esbuild emits the ESM entry's exports as one trailing `export { a as B, ... }` + * statement. Requiring exactly one is what keeps this from under-reading: a + * second export form would otherwise go unnoticed. + */ +const esModuleExportsOf = (file: string): Set => { + const statements = [...fs.readFileSync(file, 'utf8').matchAll(/\bexport\s*\{([^{}]*)\}/g)]; + if (statements.length !== 1) { + throw new Error(`${file}: expected one export statement, found ${statements.length}`); + } + return new Set( + statements[0][1] + .split(',') + .map((clause) => clause.trim()) + .filter(Boolean) + .map((clause) => clause.split(/\s+as\s+/).pop() as string) + ); +}; + +const missingFrom = (expected: Set, actual: Set): string[] => + [...expected].filter((name) => !actual.has(name)).sort(); + +describe('declared and runtime exports agree', () => { + describe('the source tree', () => { + it('has no module path answered by both a .ts and a .tsx file', () => { + // Names each pair, so the failure is the remediation: merge the two + // files into one and delete the other. + expect(ambiguousModulesIn(sourceFiles(SRC))).toEqual([]); + }); + + it('detects the shape that caused the drift', () => { + expect( + ambiguousModulesIn(['custom/index.ts', 'custom/index.tsx', 'custom/Foo.tsx', 'index.tsx']) + ).toEqual(['custom/index: custom/index.ts, custom/index.tsx']); + }); + }); + + const built = [DTS, CJS, ESM].every((file) => fs.existsSync(file)); + + // Skipping is right for `jest` on its own, before a build. In CI it is not: + // `node-checks.yml` runs `make build` before `make tests`, so a missing bundle + // means the build stopped emitting one and every check below would otherwise + // pass by doing nothing. + it('has the built bundles to compare when running in CI', () => { + if (!process.env.CI) return; + expect(built).toBe(true); + }); + + (built ? describe : describe.skip)('dist/', () => { + // `describe.skip` still evaluates its body, so these are computed lazily + // rather than read at collection time. + let declared: { values: Set; all: Set }; + let commonJs: Set; + let esModule: Set; + + beforeAll(() => { + declared = declaredExportsOf(DTS); + commonJs = commonJsExportsOf(CJS); + esModule = esModuleExportsOf(ESM); + }, 120_000); + + it('reads a plausible number of exports from every bundle', () => { + // Guards the readers themselves: one that quietly stops matching reports + // the same "nothing missing" as bundles that genuinely agree. + for (const names of [declared.values, commonJs, esModule]) { + expect(names.size).toBeGreaterThan(500); + } + expect(declared.all.size).toBeGreaterThan(declared.values.size); + }); + + it('ships every value the declarations promise', () => { + expect(missingFrom(declared.values, commonJs)).toEqual([]); + }); + + it('declares every value the runtime ships', () => { + expect(missingFrom(commonJs, declared.values)).toEqual([]); + }); + + it('ships the same values from the CommonJS and ES module bundles', () => { + expect(missingFrom(commonJs, esModule)).toEqual([]); + expect(missingFrom(esModule, commonJs)).toEqual([]); + }); + + it.each([ + 'HelperTextPopover', + 'RenderMarkdownTooltip', + 'L5DeleteIcon', + 'L5EditIcon', + 'TooltipIcon' + ])('declares and ships %s, which drifted in 0.22.x', (name) => { + expect(declared.values).toContain(name); + expect(commonJs).toContain(name); + }); + }); +}); diff --git a/src/__testing__/publishedTypeSurfaceDependencies.test.ts b/src/__testing__/publishedTypeSurfaceDependencies.test.ts index 5191646fd..a429662ae 100644 --- a/src/__testing__/publishedTypeSurfaceDependencies.test.ts +++ b/src/__testing__/publishedTypeSurfaceDependencies.test.ts @@ -120,6 +120,17 @@ const optionalPeersOf = (pkg: PackageJson): string[] => .filter(([, meta]) => meta?.optional) .map(([name]) => name); +/** + * `@types/foo` -> `foo`, `@types/scope__name` -> `@scope/name`: the module a + * DefinitelyTyped package supplies declarations for, following TypeScript's own + * mangling of scoped names. `undefined` for any package that is not one. + */ +const typedModuleOf = (name: string): string | undefined => { + if (!name.startsWith('@types/')) return undefined; + const typed = name.slice('@types/'.length); + return typed.includes('__') ? `@${typed.replace('__', '/')}` : typed; +}; + /** * What a consumer is *guaranteed* to have: dependencies, plus the peers npm * installs for them - which excludes every peer marked optional. @@ -127,14 +138,24 @@ const optionalPeersOf = (pkg: PackageJson): string[] => * Read from `peerDependenciesMeta` rather than a hardcoded list, so marking a * new peer optional immediately tightens this guard instead of leaving a hole * that only shows up in a downstream install. + * + * A declared `@types/foo` also counts as having `foo` for this purpose: the + * declaration bundle's `from 'foo'` is a type reference, and TypeScript resolves + * it through `@types/foo`. That is how `mui-datatables` is typed - the runtime is + * sistent's untyped `@sistent/mui-datatables` fork, the types are upstream's. */ const installedByConsumersOf = (pkg: PackageJson): string[] => { const optional = optionalPeersOf(pkg); - return [ + const installed = [ ...Object.keys(pkg.dependencies ?? {}), ...Object.keys(pkg.peerDependencies ?? {}) ].filter((name) => !optional.includes(name)); + + return [ + ...installed, + ...installed.map(typedModuleOf).filter((name): name is string => name !== undefined) + ]; }; describe('the published type surface only names packages a consumer installs', () => { @@ -259,6 +280,25 @@ describe('the published type surface only names packages a consumer installs', ( expect(installedByConsumersOf(pkg)).not.toContain('typescript'); }); + it('counts the module a declared @types package supplies declarations for', () => { + const typed: PackageJson = { + dependencies: { '@types/mui-datatables': '*', '@types/scope__name': '*' } + }; + + expect(installedByConsumersOf(typed)).toEqual([ + '@types/mui-datatables', + '@types/scope__name', + 'mui-datatables', + '@scope/name' + ]); + }); + + it('does not count the module behind a @types devDependency', () => { + expect( + installedByConsumersOf({ devDependencies: { '@types/js-yaml': '*' } }) + ).not.toContain('js-yaml'); + }); + it('treats a peer with no meta entry as required', () => { expect(installedByConsumersOf({ peerDependencies: { react: '^19.0.0' } })).toEqual([ 'react' diff --git a/src/custom/CatalogDesignTable/CatalogDesignTable.tsx b/src/custom/CatalogDesignTable/CatalogDesignTable.tsx index fe518302b..04a03e040 100644 --- a/src/custom/CatalogDesignTable/CatalogDesignTable.tsx +++ b/src/custom/CatalogDesignTable/CatalogDesignTable.tsx @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { MUIDataTableColumn } from '@sistent/mui-datatables'; import _ from 'lodash'; +import type { MUIDataTableColumn, MUIDataTableOptions, MUISortOptions } from 'mui-datatables'; import { useCallback, useMemo, useRef, useState } from 'react'; import { PublishIcon } from '../../icons'; import { CHARCOAL } from '../../theme'; @@ -26,13 +26,13 @@ interface CatalogDesignsTableProps { columnVisibility: Record; colViews: ColView[]; rowsPerPageOptions?: number[]; - handleBulkDeleteModal: (patterns: Pattern[], modalRef: React.RefObject) => void; + handleBulkDeleteModal: (patterns: Pattern[], modalRef: React.RefObject) => void; setSearch?: (search: string) => void; tableBackgroundColor?: string; handleBulkpatternsDataUnpublishModal: ( selected: any, patterns: Pattern[], - modalRef: React.RefObject + modalRef: React.RefObject ) => void; } @@ -97,7 +97,7 @@ export const CatalogDesignsTable: React.FC = ({ [columns, setPage, setSearch, setPageSize, setSortOrder, sortOrder] ); - const options = useMemo( + const options = useMemo( () => ({ selectableRows: _.isNil(filter) ? 'none' : 'multiple', serverSide: true, @@ -109,7 +109,7 @@ export const CatalogDesignsTable: React.FC = ({ elevation: 0, sortOrder: { name: sortOrder.split(' ')[0], - direction: sortOrder.split(' ')[1] + direction: sortOrder.split(' ')[1] as MUISortOptions['direction'] }, onTableChange: handleTableChange, customToolbarSelect: _.isNil(filter) diff --git a/src/custom/CatalogDesignTable/DesignTableColumnConfig.tsx b/src/custom/CatalogDesignTable/DesignTableColumnConfig.tsx index 9a0ef8748..e494dee7f 100644 --- a/src/custom/CatalogDesignTable/DesignTableColumnConfig.tsx +++ b/src/custom/CatalogDesignTable/DesignTableColumnConfig.tsx @@ -1,6 +1,6 @@ import { Lock, Public } from '@mui/icons-material'; import { Theme } from '@mui/material'; -import { MUIDataTableColumn, MUIDataTableMeta } from '@sistent/mui-datatables'; +import type { MUIDataTableColumn, MUIDataTableMeta } from 'mui-datatables'; import { Typography } from '../../base'; import { PLAYGROUND_MODES } from '../../constants/constants'; import { ChainIcon, CopyIcon, KanvasIcon, MoveFileIcon, PublishIcon } from '../../icons'; diff --git a/src/custom/CatalogDesignTable/columnConfig.tsx b/src/custom/CatalogDesignTable/columnConfig.tsx index ba4fe3ac5..38bb40ef8 100644 --- a/src/custom/CatalogDesignTable/columnConfig.tsx +++ b/src/custom/CatalogDesignTable/columnConfig.tsx @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { MUIDataTableColumn, MUIDataTableMeta } from '@sistent/mui-datatables'; +import type { MUIDataTableColumn, MUIDataTableMeta } from 'mui-datatables'; import { FacebookShareButton, LinkedinShareButton, TwitterShareButton } from 'react-share'; import { iconMedium } from '../../constants/iconsSizes'; import { diff --git a/src/custom/CustomCatalog/Helper.ts b/src/custom/CustomCatalog/Helper.ts index ce1ac7126..8f1d14072 100644 --- a/src/custom/CustomCatalog/Helper.ts +++ b/src/custom/CustomCatalog/Helper.ts @@ -47,7 +47,11 @@ const getValidSvgPaths = async ( const getWorkingVersionFromPatternFile = (patternFileContent: string) => { try { const patternFile = jsyaml.load(patternFileContent); - return patternFile?.version || DEFAULT_DESIGN_VERSION; + const version = + patternFile && typeof patternFile === 'object' && 'version' in patternFile + ? patternFile.version + : undefined; + return version ? String(version) : DEFAULT_DESIGN_VERSION; } catch (e) { console.error('Failed to parse pattern file to get version:', e); return DEFAULT_DESIGN_VERSION; @@ -71,11 +75,9 @@ export const handleImage = async ({ setAvailableTechnologies(validSvgPaths); }; -// `sanitizeCatalogImageUrl` is defined in a dependency-free leaf module so -// `src/index.tsx` can re-export it explicitly for the bundled d.ts without -// dragging this module's untyped `js-yaml` import into the declaration build -// (which would crash rollup-plugin-dts). Re-exported here to keep the existing -// `from './Helper'` import path working for internal consumers. +// `sanitizeCatalogImageUrl` lives in a dependency-free leaf module; re-exported +// here to keep the existing `from './Helper'` import path working for internal +// consumers. export { sanitizeCatalogImageUrl } from './sanitizeCatalogImageUrl'; export const DEFAULT_DESIGN_VERSION = '0.0.0'; diff --git a/src/custom/CustomCatalog/sanitizeCatalogImageUrl.ts b/src/custom/CustomCatalog/sanitizeCatalogImageUrl.ts index 4a2b7d905..bd0de1651 100644 --- a/src/custom/CustomCatalog/sanitizeCatalogImageUrl.ts +++ b/src/custom/CustomCatalog/sanitizeCatalogImageUrl.ts @@ -9,13 +9,6 @@ * preserving genuine `%XX` percent-encoding) and returns the result only when * it is an absolute http(s) URL; otherwise it returns `undefined` so callers * can fall back to a placeholder instead of requesting a malformed URL. - * - * This lives in its own dependency-free leaf module (rather than inline in - * `Helper.ts`) so `src/index.tsx` can re-export it explicitly for the bundled - * d.ts. `Helper.ts` imports the untyped `js-yaml`, which would crash the - * rollup-plugin-dts declaration build the moment the entry re-exports from it; - * this leaf has no untyped imports. Same rationale as `TableActions` vs - * `ResponsiveDataTable`. */ export const sanitizeCatalogImageUrl = (rawUrl?: string): string | undefined => { if (typeof rawUrl !== 'string') { diff --git a/src/custom/CustomColumnVisibilityControl/CustomColumnVisibilityControl.tsx b/src/custom/CustomColumnVisibilityControl/CustomColumnVisibilityControl.tsx index bbe892073..6ba849a0d 100644 --- a/src/custom/CustomColumnVisibilityControl/CustomColumnVisibilityControl.tsx +++ b/src/custom/CustomColumnVisibilityControl/CustomColumnVisibilityControl.tsx @@ -1,4 +1,4 @@ -import { MUIDataTableColumn } from '@sistent/mui-datatables'; +import type { MUIDataTableColumn } from 'mui-datatables'; import React from 'react'; import { Box } from '../../base/Box'; import { Card } from '../../base/Card'; diff --git a/src/custom/ErrorBoundary/ErrorBoundary.tsx b/src/custom/ErrorBoundary/ErrorBoundary.tsx index a47c3e8cb..0132e308f 100644 --- a/src/custom/ErrorBoundary/ErrorBoundary.tsx +++ b/src/custom/ErrorBoundary/ErrorBoundary.tsx @@ -1,6 +1,6 @@ import { styled } from '@mui/material'; import React from 'react'; -import { FallbackProps, ErrorBoundary as ReactErrorBoundary } from 'react-error-boundary'; +import { ErrorBoundary as ReactErrorBoundary } from 'react-error-boundary'; import { Box } from '../../base/Box'; import { Link } from '../../base/Link'; import { Typography } from '../../base/Typography'; @@ -29,7 +29,19 @@ const CodeMessage = styled('div')(({ theme }) => ({ marginBlock: '.5rem' })); -interface FallbackComponentProps extends FallbackProps { +/** + * What a fallback component receives. Declared here rather than taken from + * `react-error-boundary`, which is bundled into the runtime and is not installed + * by consumers, so naming its `FallbackProps` in the published declarations + * would reach them as `any`. Passing a fallback to `ReactErrorBoundary` below + * still checks this against the library's own type at build time. + */ +export type ErrorFallbackProps = { + error: unknown; + resetErrorBoundary: (...args: unknown[]) => void; +}; + +interface FallbackComponentProps extends ErrorFallbackProps { resetErrorBoundary: () => void; children?: React.ReactNode; pageUrl?: string; @@ -75,7 +87,7 @@ export function Fallback({ ); } -const reportError = (error: Error, info: React.ErrorInfo): void => { +const reportError = (error: unknown, info: React.ErrorInfo): void => { const pageUrl = window.location.href; const timestamp = new Date().toLocaleString(); // This is where you'd send the error to Sentry, etc @@ -92,7 +104,7 @@ const reportError = (error: Error, info: React.ErrorInfo): void => { }; interface ErrorBoundaryProps { - customFallback?: React.ComponentType; + customFallback?: React.ComponentType; children: React.ReactNode; onErrorCaught?: (error: string) => void; } @@ -105,9 +117,11 @@ export const ErrorBoundary: React.FC = ({ const pageUrl = window.location.href; const timestamp = new Date().toLocaleString(); - const handleError = (error: Error, info: React.ErrorInfo) => { + // react-error-boundary types the caught value as `unknown`: anything can be + // thrown, not only an `Error`. + const handleError = (error: unknown, info: React.ErrorInfo) => { // Pass error message to onErrorCaught - onErrorCaught?.(error.message); + onErrorCaught?.(error instanceof Error ? error.message : String(error)); reportError(error, info); }; diff --git a/src/custom/ErrorBoundary/index.tsx b/src/custom/ErrorBoundary/index.tsx index f4b09c8c0..4395c8a21 100644 --- a/src/custom/ErrorBoundary/index.tsx +++ b/src/custom/ErrorBoundary/index.tsx @@ -2,5 +2,6 @@ export { ErrorBoundary, Fallback, withErrorBoundary, - withSuppressedErrorBoundary + withSuppressedErrorBoundary, + type ErrorFallbackProps } from './ErrorBoundary'; diff --git a/src/custom/FlipCard/FlipCard.tsx b/src/custom/FlipCard/FlipCard.tsx index 97aea50cf..e6d4d9fb0 100644 --- a/src/custom/FlipCard/FlipCard.tsx +++ b/src/custom/FlipCard/FlipCard.tsx @@ -80,7 +80,7 @@ export function FlipCard({ const [flipped, setFlipped] = React.useState(false); const [activeBack, setActiveBack] = React.useState(false); - const timeout = React.useRef(null); + const timeout = React.useRef>(null); const Front = GetChild(children, 0); const Back = GetChild(children, 1); diff --git a/src/custom/Helpers/Notification/index.tsx b/src/custom/Helpers/Notification/index.tsx index 01c24112f..43be2e8f9 100644 --- a/src/custom/Helpers/Notification/index.tsx +++ b/src/custom/Helpers/Notification/index.tsx @@ -1,3 +1,3 @@ -import useNotificationHandler from './notification-handler'; +import useNotificationHandler, { type NotificationHandlerOptions } from './notification-handler'; -export { useNotificationHandler }; +export { useNotificationHandler, type NotificationHandlerOptions }; diff --git a/src/custom/Helpers/Notification/notification-handler.ts b/src/custom/Helpers/Notification/notification-handler.ts index 446011303..623ceeea0 100644 --- a/src/custom/Helpers/Notification/notification-handler.ts +++ b/src/custom/Helpers/Notification/notification-handler.ts @@ -1,7 +1,22 @@ -import { OptionsObject, useSnackbar } from 'notistack'; +import { useSnackbar } from 'notistack'; import React from 'react'; -type NotificationHandler = (message: string, options?: OptionsObject) => void; +/** + * The snackbar options this handler forwards. A subset declared here rather + * than notistack's `OptionsObject`: notistack is bundled into the runtime and is + * not installed by consumers, so naming its types in the published declarations + * would reach them as `any`. Forwarding to `enqueueSnackbar` below still checks + * this against notistack's own type at build time. + */ +export type NotificationHandlerOptions = { + key?: string | number; + variant?: 'default' | 'error' | 'success' | 'warning' | 'info'; + autoHideDuration?: number | null; + persist?: boolean; + preventDuplicate?: boolean; +}; + +type NotificationHandler = (message: string, options?: NotificationHandlerOptions) => void; const useNotificationHandler = (): NotificationHandler => { const [message, setMessage] = React.useState(''); diff --git a/src/custom/ResourceDetailFormatters/Formatter.tsx b/src/custom/ResourceDetailFormatters/Formatter.tsx index d9ec2eeda..6c93da6c8 100644 --- a/src/custom/ResourceDetailFormatters/Formatter.tsx +++ b/src/custom/ResourceDetailFormatters/Formatter.tsx @@ -2,6 +2,7 @@ import VisibilityIcon from '@mui/icons-material/Visibility'; import VisibilityOffIcon from '@mui/icons-material/VisibilityOff'; import { ChartOptions } from 'billboard.js'; import _ from 'lodash'; +import type { MUIDataTableOptions } from 'mui-datatables'; import React, { useCallback, useContext, useMemo } from 'react'; import { Box, Chip, Collapse, Grid2, IconButton, Typography } from '../../base'; @@ -487,7 +488,7 @@ export const TableDataFormatter: React.FC = ({ tableData = [Object.values(data)]; } } - const options = { + const options: MUIDataTableOptions = { filter: false, download: false, print: false, diff --git a/src/custom/ResponsiveDataTable.tsx b/src/custom/ResponsiveDataTable.tsx index 54eb93311..8ff339333 100644 --- a/src/custom/ResponsiveDataTable.tsx +++ b/src/custom/ResponsiveDataTable.tsx @@ -1,4 +1,5 @@ -import MUIDataTable, { MUIDataTableColumn, MUIDataTableOptions } from '@sistent/mui-datatables'; +import MUIDataTable from '@sistent/mui-datatables'; +import type { MUIDataTableColumn, MUIDataTableOptions } from 'mui-datatables'; import React, { useCallback } from 'react'; import { Checkbox, Collapse, ListItemIcon, ListItemText, Menu, MenuItem } from '../base'; import { ShareIcon } from '../icons'; @@ -161,11 +162,9 @@ const ResponsiveDataTable = ({ }: ResponsiveDataTableProps): JSX.Element => { const textLabels = options?.textLabels || {}; const bodyTextLabels = textLabels.body || {}; - + const noMatchMessage = - typeof bodyTextLabels.noMatch === 'string' - ? bodyTextLabels.noMatch - : 'No data available'; + typeof bodyTextLabels.noMatch === 'string' ? bodyTextLabels.noMatch : 'No data available'; const updatedOptions = { ...options, diff --git a/src/custom/TableActions.tsx b/src/custom/TableActions.tsx index 2549760f3..1a2092a37 100644 --- a/src/custom/TableActions.tsx +++ b/src/custom/TableActions.tsx @@ -3,14 +3,6 @@ import { CopyLinkIcon } from '../icons'; /** * Descriptor for a single entry in a `ResponsiveDataTable` row-action menu * (`Column.options.actionsList`). - * - * Defined in this leaf module (rather than inline in `ResponsiveDataTable`) so - * the type and the `getCopyDeepLinkAction` helper can be re-exported explicitly - * from the package root. `ResponsiveDataTable` imports `@sistent/mui-datatables`, - * which ships no type declarations; re-exporting directly from it makes - * rollup-plugin-dts resolve that untyped import and drop the whole declaration - * bundle. Keeping these here, free of untyped imports, lets them survive into - * the published `.d.ts` without a consumer-side augmentation. */ export type TableAction = { title: string; diff --git a/src/custom/TeamTable/TeamTable.tsx b/src/custom/TeamTable/TeamTable.tsx index 1fd93bf1e..3ccfd2287 100644 --- a/src/custom/TeamTable/TeamTable.tsx +++ b/src/custom/TeamTable/TeamTable.tsx @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { TableCell } from '@mui/material'; -import { MUIDataTableColumn } from '@sistent/mui-datatables'; +import type { MUIDataTableColumn } from 'mui-datatables'; import { Grid2 } from '../../base'; import { styled, useTheme } from '../../theme'; import { ErrorBoundary } from '../ErrorBoundary/ErrorBoundary.js'; diff --git a/src/custom/TeamTable/TeamTableConfiguration.tsx b/src/custom/TeamTable/TeamTableConfiguration.tsx index 8221be154..f2218c849 100644 --- a/src/custom/TeamTable/TeamTableConfiguration.tsx +++ b/src/custom/TeamTable/TeamTableConfiguration.tsx @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { MUIDataTableColumn, MUIDataTableMeta } from '@sistent/mui-datatables'; +import type { MUIDataTableColumn, MUIDataTableMeta } from 'mui-datatables'; import { useState } from 'react'; import { DeleteIcon, EditIcon } from '../../icons'; import LogoutIcon from '../../icons/Logout/LogOutIcon'; diff --git a/src/custom/TransferModal/TransferList/TransferList.tsx b/src/custom/TransferModal/TransferList/TransferList.tsx index c7b970e7e..d06c37998 100644 --- a/src/custom/TransferModal/TransferList/TransferList.tsx +++ b/src/custom/TransferModal/TransferList/TransferList.tsx @@ -298,7 +298,7 @@ function TransferList({ {customList(left, emptyStateIconLeft, emtyStateMessageLeft, 'leftList')} - + = ({ return ( <> - id="user-search-field" sx={{ width: '100%' }} // eslint-disable-next-line @typescript-eslint/ban-ts-comment diff --git a/src/custom/UsersTable/UsersTable.tsx b/src/custom/UsersTable/UsersTable.tsx index aec6c69d5..04d818b6b 100644 --- a/src/custom/UsersTable/UsersTable.tsx +++ b/src/custom/UsersTable/UsersTable.tsx @@ -1,6 +1,11 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { Theme } from '@mui/material'; -import { MUIDataTableColumn, MUIDataTableMeta } from '@sistent/mui-datatables'; +import type { + MUIDataTableColumn, + MUIDataTableMeta, + MUIDataTableOptions, + MUISortOptions +} from 'mui-datatables'; import { useRef, useState } from 'react'; import { Box, Tooltip } from '../../base'; import { EditIcon } from '../../icons'; @@ -197,9 +202,9 @@ const UsersTable: React.FC = ({ ); - let searchTimeout: NodeJS.Timeout; + let searchTimeout: ReturnType; - const options = { + const options: MUIDataTableOptions = { search: false, viewColumns: false, filter: false, @@ -215,7 +220,7 @@ const UsersTable: React.FC = ({ serverSide: true, sortOrder: { name: sortOrder.split(' ')[0], - direction: sortOrder.split(' ')[1] + direction: sortOrder.split(' ')[1] as MUISortOptions['direction'] }, onTableChange: (action: string, tableState: any) => { const sortInfo = tableState.announceText ? tableState.announceText.split(' : ') : []; diff --git a/src/custom/Workspaces/EnvironmentTable.tsx b/src/custom/Workspaces/EnvironmentTable.tsx index 757eb17c0..8c2ea09b4 100644 --- a/src/custom/Workspaces/EnvironmentTable.tsx +++ b/src/custom/Workspaces/EnvironmentTable.tsx @@ -1,6 +1,11 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; -import { MUIDataTableColumn, MUIDataTableMeta } from '@sistent/mui-datatables'; +import type { + MUIDataTableColumn, + MUIDataTableMeta, + MUIDataTableOptions, + MUISortOptions +} from 'mui-datatables'; import React, { useState } from 'react'; import { Accordion, AccordionDetails, AccordionSummary, Typography } from '../../base'; import { DeleteIcon, EnvironmentIcon } from '../../icons'; @@ -197,7 +202,7 @@ const EnvironmentTable: React.FC = ({ return initialVisibility; }); - const options = { + const options: MUIDataTableOptions = { filter: false, responsive: 'standard', selectableRows: 'none', @@ -207,7 +212,7 @@ const EnvironmentTable: React.FC = ({ elevation: 0, sortOrder: { name: sortOrder.split(' ')[0], - direction: sortOrder.split(' ')[1] + direction: sortOrder.split(' ')[1] as MUISortOptions['direction'] }, serverSide: true, onTableChange: (action: string, tableState: any) => { diff --git a/src/custom/Workspaces/WorkspaceViewsTable.tsx b/src/custom/Workspaces/WorkspaceViewsTable.tsx index aef3b37c8..b7928ba79 100644 --- a/src/custom/Workspaces/WorkspaceViewsTable.tsx +++ b/src/custom/Workspaces/WorkspaceViewsTable.tsx @@ -1,7 +1,12 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { Lock, Public } from '@mui/icons-material'; import RemoveCircleIcon from '@mui/icons-material/RemoveCircle'; -import { MUIDataTableColumn, MUIDataTableMeta } from '@sistent/mui-datatables'; +import type { + MUIDataTableColumn, + MUIDataTableMeta, + MUIDataTableOptions, + MUISortOptions +} from 'mui-datatables'; import React, { useState } from 'react'; import { Box } from '../../base'; import { EnvironmentIcon } from '../../icons'; @@ -288,7 +293,7 @@ const WorkspaceViewsTable: React.FC = ({ return initialVisibility; }); - const options = { + const options: MUIDataTableOptions = { filter: false, responsive: 'standard', selectableRows: 'none', @@ -299,7 +304,7 @@ const WorkspaceViewsTable: React.FC = ({ elevation: 0, sortOrder: { name: sortOrder.split(' ')[0], - direction: sortOrder.split(' ')[1] + direction: sortOrder.split(' ')[1] as MUISortOptions['direction'] }, onTableChange: (action: string, tableState: any) => { const sortInfo = tableState.announceText ? tableState.announceText.split(' : ') : []; diff --git a/src/custom/index.ts b/src/custom/index.ts deleted file mode 100644 index bfc9cd185..000000000 --- a/src/custom/index.ts +++ /dev/null @@ -1,10 +0,0 @@ -// Export all custom components -export * from './CustomTooltip'; -export * from './DashboardLayout'; -export * from './HelperTextPopover'; -export * from './Markdown'; -export * from './Modal'; -export * from './RJSFFormWrapper'; -export * from './StyledAccordion'; -export * from './WidgetPicker'; -export * from './WidgetEmptyState'; diff --git a/src/custom/index.tsx b/src/custom/index.tsx index bff31634c..df468b8a7 100644 --- a/src/custom/index.tsx +++ b/src/custom/index.tsx @@ -31,6 +31,7 @@ import { CustomDialogProps } from './Dialog/CustomDialog'; import { EmptyState } from './EmptyState'; import { ErrorBoundary, + type ErrorFallbackProps, Fallback, withErrorBoundary, withSuppressedErrorBoundary @@ -39,7 +40,7 @@ import { FeedbackButton } from './Feedback'; import { FlipCard, FlipCardProps } from './FlipCard'; import { FormatId } from './FormatId'; import { useWindowDimensions } from './Helpers/Dimension'; -import { useNotificationHandler } from './Helpers/Notification'; +import { type NotificationHandlerOptions, useNotificationHandler } from './Helpers/Notification'; import { ColView, updateVisibleColumns } from './Helpers/ResponsiveColumns/responsive-coulmns.tsx'; import { LearningCard } from './LearningCard'; import { BasicMarkdown, RenderMarkdown } from './Markdown'; @@ -172,8 +173,10 @@ export type { CustomColumnVisibilityControlProps, CustomDialogProps, DataTableToolbarProps, + ErrorFallbackProps, FlipCardProps, IPopperListener, + NotificationHandlerOptions, ResponsiveDataTableProps, SearchBarProps, StyledCardProps, @@ -184,12 +187,20 @@ export type { export * from './CatalogDesignTable'; export * from './CatalogDetail'; +export * from './CustomTooltip'; +export * from './DashboardLayout'; export * from './DashboardWidgets'; export * from './Dialog'; +export * from './HelperTextPopover'; export * from './LiquidGlass'; +export * from './Markdown'; +export * from './Modal'; export * from './permissions'; export * from './ResourceDetailFormatters'; export * from './RJSFFormWrapper'; export * from './ShareModal'; +export * from './StyledAccordion'; export * from './UserSearchField'; +export * from './WidgetEmptyState'; +export * from './WidgetPicker'; export * from './Workspaces'; diff --git a/src/icons/Chain/index.tsx b/src/icons/Chain/index.tsx deleted file mode 100644 index 043710db5..000000000 --- a/src/icons/Chain/index.tsx +++ /dev/null @@ -1 +0,0 @@ -export { default as ChainIcon } from './ChainIcon'; diff --git a/src/index.tsx b/src/index.tsx index 24a87e1a1..b098906ca 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -9,40 +9,23 @@ export * from './schemas'; export * from './theme'; export * from './utils'; -// FeedbackButton's type is dropped from the bundled d.ts when it reaches the -// entry only through `export * from './custom'`: rollup-plugin-dts (used by -// tsup for the declaration bundle) fails to propagate certain re-exports -// through nested barrels, so `import { FeedbackButton } from "@sistent/sistent"` -// fails type-checking even though the runtime export exists. An explicit -// re-export forces the declaration into the published bundle. The same quirk -// affects other custom components (see consumers' local d.ts augmentations); -// add them here as they are needed. -export { FeedbackButton, type FeedbackComponentProps } from './custom/Feedback'; -// `TableAction` and `getCopyDeepLinkAction` live in the leaf `TableActions` -// module (not `ResponsiveDataTable`, which imports the untyped -// `@sistent/mui-datatables` and would crash the dts build) precisely so this -// explicit re-export can force them into the published declaration bundle. -export { getCopyDeepLinkAction, type TableAction } from './custom/TableActions'; -// Same nested-barrel dts-drop quirk as FeedbackButton above: without this -// explicit re-export the DangerConfirmationModal declarations (and its exported -// props types) are dropped from the bundled d.ts, breaking -// `import { DangerConfirmationModal } from "@sistent/sistent"` type-checking. +// The explicit re-exports below were added to work around what looked like +// rollup-plugin-dts dropping declarations reached through `export * from +// './custom'`. The real cause was `src/custom/` having both an `index.ts` and an +// `index.tsx` barrel, which the runtime and declaration builds resolve in +// opposite orders; with one barrel the `export *` above carries everything, and +// `declarationRuntimeExportParity.test.ts` keeps the two builds in step. They +// stay because some also export names the domain barrels do not (e.g. +// `FeedbackComponentProps`, `TeamPickerRecord`). export { DangerConfirmationModal, type DangerConfirmationCheckbox, type DangerConfirmationModalProps } from './custom/DangerConfirmationModal'; +export { FeedbackButton, type FeedbackComponentProps } from './custom/Feedback'; +export { getCopyDeepLinkAction, type TableAction } from './custom/TableActions'; -export { - DashboardLayout, - type DashboardLayoutProps -} from './custom/DashboardLayout'; -// Same nested-barrel dts-drop quirk as FeedbackButton above: UniversalFilter -// (and its FilterColumn / UniversalFilterProps types) reaches the entry only -// through `export * from './custom'`, so rollup-plugin-dts drops it from the -// bundled d.ts and `import { UniversalFilter } from "@sistent/sistent"` fails -// type-checking despite the runtime export. The explicit re-export forces the -// declaration into the published bundle. +export { DashboardLayout, type DashboardLayoutProps } from './custom/DashboardLayout'; export { default as UniversalFilter, type DateRange, @@ -53,21 +36,12 @@ export { export { DataTableToolbar, type DataTableToolbarProps } from './custom/DataTableToolbar'; -// Same nested-barrel dts-drop quirk as FeedbackButton above: reaching the entry -// only through `export * from './custom'`, rollup-plugin-dts drops both the -// `NavigationNavbar` component declaration and the `NavigationItem` type from the -// bundled d.ts, so `import { NavigationNavbar, type NavigationItem } from -// "@sistent/sistent"` fails type-checking despite the runtime exports existing. -// The explicit re-export forces both declarations into the published bundle. export { NavigationNavbar, type NavigationItem } from './custom/NavigationNavbar'; -// Same nested-barrel dts-drop quirk as FeedbackButton above. `createCanShow` is -// worse than a missing type: consumers still resolve it at runtime, so the import -// silently degrades to `any` and its `eventBus` argument stops being -// variance-checked - the one place a host hands its event bus to sistent. -// Its parameter types travel with it: a consumer that cannot name `HasKeyProps` -// or `ReasonEventPublisher` cannot type the wrapper it builds around the -// returned component, and falls straight back to `any`. +// `createCanShow` takes the host's event bus, so its parameter types travel with +// it: a consumer that cannot name `HasKeyProps` or `ReasonEventPublisher` cannot +// type the wrapper it builds around the returned component, and falls straight +// back to `any`. export { createCanShow, type HasKeyProps, @@ -99,40 +73,25 @@ export { export { useAccessibleOrgs, - type UseAccessibleOrgsOptions, - type TriggerGetKeys + type TriggerGetKeys, + type UseAccessibleOrgsOptions } from './hooks/useAccessibleOrgs'; -export { - WidgetPicker, - type WidgetPickerProps, - type WidgetItem -} from './custom/WidgetPicker'; +export { WidgetPicker, type WidgetItem, type WidgetPickerProps } from './custom/WidgetPicker'; -export { - WidgetEmptyState, - type WidgetEmptyStateProps -} from './custom/WidgetEmptyState'; +export { WidgetEmptyState, type WidgetEmptyStateProps } from './custom/WidgetEmptyState'; export { BottomSheet, type BottomSheetProps } from './custom/BottomSheet'; export { ActionButton, type ActionButtonProps, type Option } from './custom/ActionButton'; -// Same nested-barrel dts-drop quirk as FeedbackButton above. The share/revoke -// payload builders exist so that hosts stop hand-rolling the +// The share/revoke payload builders exist so that hosts stop hand-rolling the // `resourceAccessMappingPayload` body they hand to `ShareModal`'s // `resourceAccessMutator`: the server drops unrecognised keys silently and // still answers 200, so a hand-rolled body fails as a successful no-op. // Without the declarations a host cannot type that body at all and falls back // to the object literal that caused the bug. // -// The component is dropped by the same quirk, which left hosts able to type the -// share body but not the component consuming it. This is one instance of a -// wider gap - most of `src/custom/` reaches `dist/index.js` without reaching -// `dist/index.d.ts` - so a per-symbol line here is a stopgap, not the fix. -// AGENTS.md, "New public exports need an explicit root re-export", owns the -// measurement and the command that reproduces it. -// // Taken from the `./custom/ShareModal` barrel rather than the leaf // `resourceAccessPayload` module: that barrel re-exports the builders as well // as the component, so one statement is the single source of truth for where @@ -149,10 +108,7 @@ export { type ShareModalProps } from './custom/ShareModal'; -// Same nested-barrel dts-drop quirk as FeedbackButton above, and the whole -// `DashboardWidgets` barrel is subject to it - `TeamSearchField` reaches -// `dist/index.js` but not `dist/index.d.ts`, so a host cannot name the -// component at all. Its two prop types travel with it: `teamsData` and +// `TeamSearchField`'s two prop types travel with it: `teamsData` and // `setTeamsData` are both keyed on the team-picker record, so a consumer that // cannot name it cannot hold the state the component requires and falls back // to `any`. Taken from the leaf module because the barrel re-exports only the diff --git a/src/theme/components/table.modifier.ts b/src/theme/components/table.modifier.ts index d90c1ba1a..62355e043 100644 --- a/src/theme/components/table.modifier.ts +++ b/src/theme/components/table.modifier.ts @@ -89,7 +89,11 @@ const MuiTableSortLabel: Components['MuiTableSortLabel'] = { } }; -const MUIDataTableSelectCell: Components['MUIDataTableSelectCell'] = { +// `@types/mui-datatables` declares `MUIDataTableSelectCell` by augmenting the +// `@mui/material` it depends on (v5, installed nested under it), so the key never +// reaches the `Components` of the MUI this package builds against. The slot +// renders a table cell and takes the same overrides, so it is typed as one. +const MUIDataTableSelectCell: Components['MuiTableCell'] = { styleOverrides: { root: ({ theme }) => ({ backgroundColor: theme.palette.background.constant?.table || theme.palette.background.paper, diff --git a/src/types/sistent-mui-datatables.d.ts b/src/types/sistent-mui-datatables.d.ts new file mode 100644 index 000000000..d509d7f66 --- /dev/null +++ b/src/types/sistent-mui-datatables.d.ts @@ -0,0 +1,15 @@ +// `@sistent/mui-datatables` is sistent's fork of `mui-datatables` and ships no +// type declarations of its own. Its API is the upstream one, which +// `@types/mui-datatables` (a real `dependency`, so consumers have it too) types. +// +// Scope this shim to the runtime value import in `ResponsiveDataTable` only. +// Type imports name `mui-datatables` directly (`import type { MUIDataTableColumn } +// from 'mui-datatables'`): the declaration bundle keeps whatever specifier the +// source used, and only `mui-datatables` resolves in a consumer's tree - this +// ambient module is not published, so a type imported through it would reach +// consumers as `any`. +declare module '@sistent/mui-datatables' { + import MUIDataTable from 'mui-datatables'; + + export default MUIDataTable; +} From a1f6ac4686b9e9f97f3ab66ff981a41eb4ed0bff Mon Sep 17 00:00:00 2001 From: Michelle Jameson <241148084+chellej@users.noreply.github.com> Date: Wed, 30 Sep 2026 03:53:50 -0500 Subject: [PATCH 2/5] no-mistakes(review): Fix notify double-enqueue, pin mui-datatables types, prune source guards Signed-off-by: Michelle Jameson <241148084+chellej@users.noreply.github.com> --- AGENTS.md | 7 ++- package-lock.json | 2 +- package.json | 2 +- src/__testing__/RJSFFormWrapper.test.tsx | 34 +++--------- .../declarationRuntimeExportParity.test.ts | 55 ++++--------------- .../useNotificationHandler.test.tsx | 41 ++++++++++++++ .../Notification/notification-handler.ts | 26 ++++----- 7 files changed, 75 insertions(+), 92 deletions(-) create mode 100644 src/__testing__/useNotificationHandler.test.tsx diff --git a/AGENTS.md b/AGENTS.md index 57a2028e4..f14bafd80 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -214,9 +214,10 @@ long misread as a `rollup-plugin-dts` "nested-barrel drop", which is why `src/in carries explicit re-exports that are no longer load-bearing. [`src/__testing__/declarationRuntimeExportParity.test.ts`](src/__testing__/declarationRuntimeExportParity.test.ts) -is the guard: it rejects any `.ts`/`.tsx` pair in `src/`, and compares the built `dist/index.d.ts` -value exports with `dist/index.js` and `dist/index.mjs` in both directions (skips without a build, -fails in CI without one - same contract as the type-surface guard above). +is the guard: it compares the built `dist/index.d.ts` value exports with `dist/index.js` and +`dist/index.mjs` in both directions (skips without a build, fails in CI without one - same contract +as the type-surface guard above). It reads only built output, so a `.ts`/`.tsx` pair surfaces there +as a mismatch after `make build`, not as a source-level error. The dts step type-checks everything reachable from `src/index.tsx`, so code newly exported must type-check under `strict`, and a package it imports must have types. `@sistent/mui-datatables` diff --git a/package-lock.json b/package-lock.json index 43285cb3d..08d5b3fe5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "@layer5/meshery-design-embed": "^0.6.0", "@meshery/schemas": "^1.3.44", "@sistent/mui-datatables": "^8.0.0", - "@types/mui-datatables": "*", + "@types/mui-datatables": "^4.3.13", "billboard.js": "^4.0.3", "js-yaml": "^5.2.3", "lodash": "^4.18.1", diff --git a/package.json b/package.json index a45f1c878..4879a19d3 100644 --- a/package.json +++ b/package.json @@ -137,7 +137,7 @@ "@layer5/meshery-design-embed": "^0.6.0", "@meshery/schemas": "^1.3.44", "@sistent/mui-datatables": "^8.0.0", - "@types/mui-datatables": "*", + "@types/mui-datatables": "^4.3.13", "billboard.js": "^4.0.3", "js-yaml": "^5.2.3", "lodash": "^4.18.1", diff --git a/src/__testing__/RJSFFormWrapper.test.tsx b/src/__testing__/RJSFFormWrapper.test.tsx index 11f25ba52..441b98246 100644 --- a/src/__testing__/RJSFFormWrapper.test.tsx +++ b/src/__testing__/RJSFFormWrapper.test.tsx @@ -2,39 +2,19 @@ * Surface-level smoke test for the RJSFFormWrapper / RJSFFormModal * exports added in layer5io/sistent#1533. * - * Two assertions: - * - * 1. The wrapper module loads cleanly with the @rjsf/* peer-deps - * installed (deep-path import). - * - * 2. The new symbols are wired through BOTH `src/custom` barrels - * (`index.ts` AND `index.tsx`). sistent ships two barrel files - * at this path — TypeScript's module resolution prefers `.ts` - * over `.tsx`, while tsup's runtime emit reads `.tsx`. If an - * export lands in only one of them, runtime and types diverge: - * the symbol appears in `dist/index.mjs` but is missing from - * `dist/index.d.ts` (or vice versa). The barrel check is done - * statically by reading the source files rather than by - * importing them at runtime, because the runtime barrel pulls - * in sistent's `Markdown` -> `react-markdown` ESM chain that - * would need a widened jest `transformIgnorePatterns`. + * This checks that the wrapper module loads cleanly with the @rjsf/* + * peer-deps installed (deep-path import). That both symbols are declared + * and shipped from the package root is pinned against the built bundles + * in `declarationRuntimeExportParity.test.ts`. */ -import * as fs from 'fs'; -import * as path from 'path'; import { RJSFFormWrapper } from '../custom/RJSFFormWrapper/RJSFFormWrapper'; describe('RJSFFormWrapper (sistent#1533)', () => { it('exports a function with stable displayName from the deep path', () => { expect(typeof RJSFFormWrapper).toBe('function'); - expect( - (RJSFFormWrapper as unknown as { displayName: string }).displayName - ).toBe('RJSFFormWrapper'); - }); - - it('src/custom/index.tsx re-exports the RJSFFormWrapper module', () => { - const full = path.resolve(__dirname, '..', '..', 'src/custom/index.tsx'); - const source = fs.readFileSync(full, 'utf8'); - expect(source).toMatch(/export\s+\*\s+from\s+['"]\.\/RJSFFormWrapper['"]/); + expect((RJSFFormWrapper as unknown as { displayName: string }).displayName).toBe( + 'RJSFFormWrapper' + ); }); }); diff --git a/src/__testing__/declarationRuntimeExportParity.test.ts b/src/__testing__/declarationRuntimeExportParity.test.ts index 6825cb939..a6fa976b0 100644 --- a/src/__testing__/declarationRuntimeExportParity.test.ts +++ b/src/__testing__/declarationRuntimeExportParity.test.ts @@ -16,9 +16,9 @@ * `index.tsx`, each a different barrel. `import './custom'` resolves by extension * order, and the two builds disagree on it - esbuild (the runtime bundle) tries * `.tsx` first, TypeScript (the declaration bundle) tries `.ts` first - so each - * build silently exported a different list. `the source tree` below rules that - * shape out before it can reach a build; the `dist/` checks catch any other way - * the two builds come to disagree. + * build silently exported a different list. The checks below compare the built + * bundles, so they catch that shape and any other way the two builds come to + * disagree. * * Runtime exports are read the way consumers read them: `cjs-module-lexer` is * what Node itself uses to expose a CommonJS module's named exports to `import`, @@ -34,38 +34,11 @@ import path from 'path'; import ts from 'typescript'; const ROOT = path.resolve(__dirname, '..', '..'); -const SRC = path.join(ROOT, 'src'); const DIST = path.join(ROOT, 'dist'); const DTS = path.join(DIST, 'index.d.ts'); const CJS = path.join(DIST, 'index.js'); const ESM = path.join(DIST, 'index.mjs'); -/** Every source file, recursively, as a path relative to `src/`. */ -const sourceFiles = (dir: string): string[] => - fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { - const full = path.join(dir, entry.name); - if (entry.isDirectory()) return sourceFiles(full); - return /\.(ts|tsx)$/.test(entry.name) && !entry.name.endsWith('.d.ts') - ? [path.relative(SRC, full)] - : []; - }); - -/** - * Module paths that more than one file answers to - `foo.ts` next to - * `foo.tsx`, which is where the two builds' resolution orders diverge. - */ -const ambiguousModulesIn = (files: string[]): string[] => { - const byModule = new Map(); - for (const file of files) { - const specifier = file.replace(/\.(ts|tsx)$/, ''); - byModule.set(specifier, [...(byModule.get(specifier) ?? []), file]); - } - return [...byModule.entries()] - .filter(([, candidates]) => candidates.length > 1) - .map(([specifier, candidates]) => `${specifier}: ${candidates.sort().join(', ')}`) - .sort(); -}; - /** Exported names of the declaration bundle, split by whether they carry a value. */ const declaredExportsOf = (dtsFile: string): { values: Set; all: Set } => { const program = ts.createProgram([dtsFile], { @@ -127,20 +100,6 @@ const missingFrom = (expected: Set, actual: Set): string[] => [...expected].filter((name) => !actual.has(name)).sort(); describe('declared and runtime exports agree', () => { - describe('the source tree', () => { - it('has no module path answered by both a .ts and a .tsx file', () => { - // Names each pair, so the failure is the remediation: merge the two - // files into one and delete the other. - expect(ambiguousModulesIn(sourceFiles(SRC))).toEqual([]); - }); - - it('detects the shape that caused the drift', () => { - expect( - ambiguousModulesIn(['custom/index.ts', 'custom/index.tsx', 'custom/Foo.tsx', 'index.tsx']) - ).toEqual(['custom/index: custom/index.ts, custom/index.tsx']); - }); - }); - const built = [DTS, CJS, ESM].every((file) => fs.existsSync(file)); // Skipping is right for `jest` on its own, before a build. In CI it is not: @@ -197,5 +156,13 @@ describe('declared and runtime exports agree', () => { expect(declared.values).toContain(name); expect(commonJs).toContain(name); }); + + it.each(['RJSFFormWrapper', 'RJSFFormModal'])( + 'declares and ships %s from the package root (sistent#1533)', + (name) => { + expect(declared.values).toContain(name); + expect(commonJs).toContain(name); + } + ); }); }); diff --git a/src/__testing__/useNotificationHandler.test.tsx b/src/__testing__/useNotificationHandler.test.tsx new file mode 100644 index 000000000..a046c98e9 --- /dev/null +++ b/src/__testing__/useNotificationHandler.test.tsx @@ -0,0 +1,41 @@ +import { act, renderHook, screen } from '@testing-library/react'; +import { SnackbarProvider } from 'notistack'; +import React from 'react'; +import { useNotificationHandler } from '../custom/Helpers/Notification'; + +const renderNotify = () => + renderHook(() => useNotificationHandler(), { + wrapper: ({ children }: { children: React.ReactNode }) => ( + {children} + ) + }).result; + +describe('useNotificationHandler', () => { + it('shows one snackbar for a call with options', () => { + const notify = renderNotify(); + + act(() => notify.current('Saved', { variant: 'success' })); + + expect(screen.getAllByText('Saved')).toHaveLength(1); + }); + + it('shows one snackbar for a call without options', () => { + const notify = renderNotify(); + + act(() => notify.current('Loaded')); + + expect(screen.getAllByText('Loaded')).toHaveLength(1); + }); + + it('shows every message when called more than once in one update', () => { + const notify = renderNotify(); + + act(() => { + notify.current('First'); + notify.current('Second'); + }); + + expect(screen.getAllByText('First')).toHaveLength(1); + expect(screen.getAllByText('Second')).toHaveLength(1); + }); +}); diff --git a/src/custom/Helpers/Notification/notification-handler.ts b/src/custom/Helpers/Notification/notification-handler.ts index 623ceeea0..29a3155cc 100644 --- a/src/custom/Helpers/Notification/notification-handler.ts +++ b/src/custom/Helpers/Notification/notification-handler.ts @@ -1,5 +1,4 @@ import { useSnackbar } from 'notistack'; -import React from 'react'; /** * The snackbar options this handler forwards. A subset declared here rather @@ -18,25 +17,20 @@ export type NotificationHandlerOptions = { type NotificationHandler = (message: string, options?: NotificationHandlerOptions) => void; +/** + * Returns a `notify` function that enqueues one snackbar per call. + * + * It reads the snackbar context of the notistack copy bundled into sistent, not + * the host's: a host's own `SnackbarProvider` comes from a different copy of + * notistack, so it does not reach this hook, and under it alone `notify` enqueues + * into notistack's default context, where nothing is shown. + */ const useNotificationHandler = (): NotificationHandler => { - const [message, setMessage] = React.useState(''); const { enqueueSnackbar } = useSnackbar(); - React.useEffect(() => { - if (message) { - enqueueSnackbar(message); - setMessage(''); - } - }, [message, enqueueSnackbar]); - - const notify: NotificationHandler = (message, options) => { - setMessage(message); - if (options) { - enqueueSnackbar(message, options); - } + return (message, options) => { + enqueueSnackbar(message, options); }; - - return notify; }; export default useNotificationHandler; From 9f367119266e1b820eab5444084be6de24bb87f1 Mon Sep 17 00:00:00 2001 From: Michelle Jameson <241148084+chellej@users.noreply.github.com> Date: Wed, 30 Sep 2026 04:16:15 -0500 Subject: [PATCH 3/5] no-mistakes(review): Keep lodash types out of dist; fix Helpers readme Signed-off-by: Michelle Jameson <241148084+chellej@users.noreply.github.com> --- .../publishedTypeSurfaceDependencies.test.ts | 55 +++++++++++++++++-- src/custom/Helpers/readme.md | 39 ++++++------- .../useResourceCleanData.ts | 3 +- 3 files changed, 73 insertions(+), 24 deletions(-) diff --git a/src/__testing__/publishedTypeSurfaceDependencies.test.ts b/src/__testing__/publishedTypeSurfaceDependencies.test.ts index a429662ae..812e63eed 100644 --- a/src/__testing__/publishedTypeSurfaceDependencies.test.ts +++ b/src/__testing__/publishedTypeSurfaceDependencies.test.ts @@ -26,10 +26,16 @@ * `devDependency` nor a peer marked optional in `peerDependenciesMeta` is part * of what a consumer necessarily installs, so neither can carry a public type - * an optional peer that is skipped fails in exactly the two ways above. + * + * The same holds for the package a named module's declarations actually come + * from. `lodash` is a `dependency` but ships no declarations; they come from + * `@types/lodash`, a `devDependency`, so naming a lodash type fails the same two + * ways even though `lodash` itself is installed. */ import fs from 'fs'; import { builtinModules } from 'module'; import path from 'path'; +import ts from 'typescript'; const ROOT = path.resolve(__dirname, '..', '..'); const DTS = path.join(ROOT, 'dist', 'index.d.ts'); @@ -103,11 +109,28 @@ const packageNameOf = (specifier: string): string => const isBare = (specifier: string): boolean => !specifier.startsWith('.') && !specifier.startsWith('/'); -const externalPackagesIn = (source: string): string[] => - [...new Set(specifiersIn(stripComments(source)).filter(isBare).map(packageNameOf))] - .filter((name) => !builtinModules.includes(name)) +const externalSpecifiersIn = (source: string): string[] => + specifiersIn(stripComments(source)) + .filter(isBare) + .filter((specifier) => !builtinModules.includes(packageNameOf(specifier))) .sort(); +const externalPackagesIn = (source: string): string[] => + [...new Set(externalSpecifiersIn(source).map(packageNameOf))].sort(); + +/** + * The package TypeScript takes `specifier`'s declarations from, resolved from + * the bundle's own location: the package itself when it ships declarations, + * its `@types/*` package when it does not, `undefined` when nothing resolves. + */ +const typesSupplierOf = (specifier: string): string | undefined => + ts.resolveModuleName( + specifier, + DTS, + { module: ts.ModuleKind.ESNext, moduleResolution: ts.ModuleResolutionKind.Bundler }, + ts.sys + ).resolvedModule?.packageId?.name; + type PackageJson = { dependencies?: Record; peerDependencies?: Record; @@ -180,7 +203,9 @@ describe('the published type surface only names packages a consumer installs', ( // its body, so an unguarded `readFileSync` here throws at collection time // and takes the whole suite down with it - turning "no build yet, nothing to // check" into a hard failure for anyone running `jest` on its own. - const referenced = built ? externalPackagesIn(fs.readFileSync(DTS, 'utf8')) : []; + const source = built ? fs.readFileSync(DTS, 'utf8') : ''; + const referenced = externalPackagesIn(source); + const specifiers = externalSpecifiersIn(source); it('references external packages at all', () => { // Guards the scan itself: a pattern that quietly stops matching reports @@ -201,6 +226,20 @@ describe('the published type surface only names packages a consumer installs', ( expect(undeclared).toEqual([]); }); + it('takes every declaration from a package a consumer installs', () => { + const exempt = [...UNDECLARED_BY_DESIGN, ...OPTIONAL_PEERS_ON_THE_RECORD]; + + const unsupplied = specifiers + .filter((specifier) => !exempt.includes(packageNameOf(specifier))) + .map((specifier) => [specifier, typesSupplierOf(specifier)] as const) + .filter( + ([, supplier]) => supplier === undefined || !installedByConsumers.includes(supplier) + ) + .map(([specifier, supplier]) => `${specifier} (declared by ${supplier ?? 'nothing'})`); + + expect(unsupplied).toEqual([]); + }); + it('carries the permission-key contract from @meshery/schemas as a real dependency', () => { // The specific regression this file was written for. `Key` is re-exported // by `src/index.tsx`, so schemas is part of sistent's public API and a @@ -257,6 +296,14 @@ describe('the published type surface only names packages a consumer installs', ( expect(externalPackagesIn(source)).toEqual(['@meshery/schemas']); }); + it.each([ + ['lodash', '@types/lodash'], + ['rxjs', 'rxjs'], + ['@meshery/schemas/permissions', '@meshery/schemas'] + ])('takes the declarations for %j from %j', (specifier, supplier) => { + expect(typesSupplierOf(specifier)).toBe(supplier); + }); + // The other half of the comparison, and the one that was wrong first: the // scan can name every external package correctly and still pass a leak // through if the set it is checked against overstates what a consumer has. diff --git a/src/custom/Helpers/readme.md b/src/custom/Helpers/readme.md index 8955df099..bc2d63d5a 100644 --- a/src/custom/Helpers/readme.md +++ b/src/custom/Helpers/readme.md @@ -13,12 +13,13 @@ This directory contains a collection of utility and helper components that you c 1. **Window Dimensions Hook**: A custom React hook for tracking changes in window dimensions. - **File**: `Dimension` - **Usage**: Provides the `useWindowDimensions` hook, which allows you to get the current window dimensions and react to changes in window size. - - **Returns**: An object containing the current window dimensions and a boolean value indicating whether the window is currently in landscape mode. + - **Returns**: An object containing the current window dimensions, `{ width, height }`. 2. **Notification Hook**: A custom React hook for displaying notifications using notistack. - **File**: `Notification` - - **Usage**: Provides the `useNotificationHandler` hook, which allows you to display notifications. - - **Returns**: An object containing the notification state and a function for updating the notification state. + - **Usage**: Provides the `useNotificationHandler` hook, which enqueues notistack snackbars. + - **Returns**: A `notify(message, options?)` function that enqueues one snackbar per call. + - **Limitation**: The hook reads the snackbar context of the notistack copy bundled into sistent, not the host's. A host's own `SnackbarProvider` comes from a different copy of notistack, so it does not reach this hook, and under it alone `notify` enqueues into notistack's default context, where nothing is shown. ## How to Use @@ -31,28 +32,28 @@ To use these helper components in your project, follow these steps: - **Example**: Importing the `useWindowDimensions` hook from the `Dimension` helper component: ```javascript - import { useWindowDimensions } from '@layer5/sistent-components'; + import { useWindowDimensions } from '@sistent/sistent'; const DimensionExample = () => { - const { width, height } = useWindowDimensions(); - - return ( -
-

Window width: {width}

-

Window height: {height}

-
- ); + const { width, height } = useWindowDimensions(); + + return ( +
+

Window width: {width}

+

Window height: {height}

+
+ ); + }; ``` - **Example**: Importing the `useNotificationHandler` hook from the `Notification` helper component: ```javascript - import useNotificationHandler from '@layer5/sistent-components'; + import { useNotificationHandler } from '@sistent/sistent'; const NotificationHandlerExample = () => { - const notify = useNotificationHandler(); + const notify = useNotificationHandler(); - return ( - - ); + return ; + }; ``` + + Under a host's own `SnackbarProvider` this example shows nothing: see the limitation above. diff --git a/src/custom/ResourceDetailFormatters/useResourceCleanData.ts b/src/custom/ResourceDetailFormatters/useResourceCleanData.ts index 3e1778f58..30687715b 100644 --- a/src/custom/ResourceDetailFormatters/useResourceCleanData.ts +++ b/src/custom/ResourceDetailFormatters/useResourceCleanData.ts @@ -152,7 +152,8 @@ export const useResourceCleanData = () => { tolerations: parsedSpec?.tolerations, podVolumes: parsedSpec?.volumes, ingressRules: parsedSpec?.rules, - connections: kind === 'Service' && _.omit(parsedSpec, ['selector', 'type']), + connections: + kind === 'Service' && (_.omit(parsedSpec, ['selector', 'type']) as Record), labels: { data: resource?.metadata?.labels?.map((label) => { const value = label?.value !== undefined ? label?.value : ''; From 809e60033fdfd7e5f7a23ae19a32d9ed92be3e37 Mon Sep 17 00:00:00 2001 From: Michelle Jameson <241148084+chellej@users.noreply.github.com> Date: Wed, 30 Sep 2026 04:59:31 -0500 Subject: [PATCH 4/5] test(ShareModal): give the wire-contract cases room for the cold start The first case renders the full ShareModal tree in a fresh worker and, under a full parallel jest run, exceeded the 5s default (seen failing the pre-commit hook while passing in isolation). Raise this file's budget to 20s; each waitFor keeps its own shorter bound. Also state in the parity-guard docblock and AGENTS.md that the dist checks compare export names only, so a .ts/.tsx pair exporting the same names from different implementations is not caught. Signed-off-by: Michelle Jameson <241148084+chellej@users.noreply.github.com> --- AGENTS.md | 5 +++-- src/__testing__/ShareModalWireContract.test.tsx | 8 ++++++++ src/__testing__/declarationRuntimeExportParity.test.ts | 8 +++++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f14bafd80..e7b237ae2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -216,8 +216,9 @@ carries explicit re-exports that are no longer load-bearing. [`src/__testing__/declarationRuntimeExportParity.test.ts`](src/__testing__/declarationRuntimeExportParity.test.ts) is the guard: it compares the built `dist/index.d.ts` value exports with `dist/index.js` and `dist/index.mjs` in both directions (skips without a build, fails in CI without one - same contract -as the type-surface guard above). It reads only built output, so a `.ts`/`.tsx` pair surfaces there -as a mismatch after `make build`, not as a source-level error. +as the type-surface guard above). It compares export *names* only, in built output: a `.ts`/`.tsx` +pair shows up there after `make build` when the two files export different names, but a pair that +exports the same names from different implementations passes. Do not add such a pair. The dts step type-checks everything reachable from `src/index.tsx`, so code newly exported must type-check under `strict`, and a package it imports must have types. `@sistent/mui-datatables` diff --git a/src/__testing__/ShareModalWireContract.test.tsx b/src/__testing__/ShareModalWireContract.test.tsx index e1222954a..03e312880 100644 --- a/src/__testing__/ShareModalWireContract.test.tsx +++ b/src/__testing__/ShareModalWireContract.test.tsx @@ -95,6 +95,14 @@ const renderShareModal = (overrides: Partial = {}) => { /** The single argument the host mutator was called with, as it would be sent. */ const sentRequest = (mutator: jest.Mock) => mutator.mock.calls[0][0]; +// Each case renders the whole ShareModal tree and drives it through the +// Autocomplete. The first case also pays the cold start (first emotion/MUI +// render in this worker), which under a full parallel `jest` run exceeds the +// 5s default and fails intermittently while passing in isolation. The budget +// is for that cold start, not for a slow assertion: each `waitFor` below keeps +// its own shorter bound. +jest.setTimeout(20_000); + describe('ShareModal resource-access wire contract', () => { it('posts the canonical grantAccess body when a person is shared with', async () => { const { resourceAccessMutator } = renderShareModal(); diff --git a/src/__testing__/declarationRuntimeExportParity.test.ts b/src/__testing__/declarationRuntimeExportParity.test.ts index a6fa976b0..e12327fce 100644 --- a/src/__testing__/declarationRuntimeExportParity.test.ts +++ b/src/__testing__/declarationRuntimeExportParity.test.ts @@ -16,9 +16,11 @@ * `index.tsx`, each a different barrel. `import './custom'` resolves by extension * order, and the two builds disagree on it - esbuild (the runtime bundle) tries * `.tsx` first, TypeScript (the declaration bundle) tries `.ts` first - so each - * build silently exported a different list. The checks below compare the built - * bundles, so they catch that shape and any other way the two builds come to - * disagree. + * build silently exported a different list. The checks below compare the + * *names* of the value exports in the built bundles, so they catch that shape + * whenever it changes which names are exported. They do not catch a `.ts`/`.tsx` + * pair whose two files export the same names from different implementations: + * the names still agree while the declared type and the shipped code do not. * * Runtime exports are read the way consumers read them: `cjs-module-lexer` is * what Node itself uses to expose a CommonJS module's named exports to `import`, From ebd203fc6d78a72feb86a412c09ab8a21a895ace Mon Sep 17 00:00:00 2001 From: Michelle Jameson <241148084+chellej@users.noreply.github.com> Date: Wed, 30 Sep 2026 05:23:44 -0500 Subject: [PATCH 5/5] fix(ErrorBoundary,notify): render any thrown value; accept snackbar position The default Fallback read `(error as Error).message`, which throws for a thrown null or undefined and drops the text of a thrown string, though react-error-boundary renders the fallback for any thrown value. Use the same conversion as the onError handler, and pin it with a test that throws an Error, a string, null and undefined. NotificationHandlerOptions also accepts `anchorOrigin`, declared locally so the published types still name no notistack type. Signed-off-by: Michelle Jameson <241148084+chellej@users.noreply.github.com> --- .../ErrorBoundaryFallback.test.tsx | 39 +++++++++++++++++++ .../useNotificationHandler.test.tsx | 10 +++++ src/custom/ErrorBoundary/ErrorBoundary.tsx | 2 +- .../Notification/notification-handler.ts | 4 ++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 src/__testing__/ErrorBoundaryFallback.test.tsx diff --git a/src/__testing__/ErrorBoundaryFallback.test.tsx b/src/__testing__/ErrorBoundaryFallback.test.tsx new file mode 100644 index 000000000..9da88059d --- /dev/null +++ b/src/__testing__/ErrorBoundaryFallback.test.tsx @@ -0,0 +1,39 @@ +import { render, screen } from '@testing-library/react'; +import { ErrorBoundary } from '../custom/ErrorBoundary'; +import { SistentThemeProvider } from '../theme'; + +// react-error-boundary renders the fallback for any thrown value, not only an +// `Error`, so the default fallback has to render every one without throwing. +const Thrower = ({ value }: { value: unknown }): JSX.Element => { + throw value; +}; + +const renderThrowing = (value: unknown) => + render( + + + + + + ); + +describe('ErrorBoundary default fallback', () => { + beforeEach(() => { + // React logs every caught render error; the boundary is the thing under test. + jest.spyOn(console, 'error').mockImplementation(() => undefined); + jest.spyOn(console, 'log').mockImplementation(() => undefined); + }); + + afterEach(() => jest.restoreAllMocks()); + + it.each([ + ['an Error', new Error('boom'), 'boom'], + ['a string', 'plain failure', 'plain failure'], + ['null', null, 'null'], + ['undefined', undefined, 'undefined'] + ])('shows the message when %s is thrown', (_label, value, shown) => { + renderThrowing(value); + + expect(screen.getByRole('alert').textContent).toContain(`Error: ${shown}`); + }); +}); diff --git a/src/__testing__/useNotificationHandler.test.tsx b/src/__testing__/useNotificationHandler.test.tsx index a046c98e9..b46ac2a06 100644 --- a/src/__testing__/useNotificationHandler.test.tsx +++ b/src/__testing__/useNotificationHandler.test.tsx @@ -27,6 +27,16 @@ describe('useNotificationHandler', () => { expect(screen.getAllByText('Loaded')).toHaveLength(1); }); + it('accepts a snackbar position', () => { + const notify = renderNotify(); + + act(() => + notify.current('Positioned', { anchorOrigin: { vertical: 'top', horizontal: 'right' } }) + ); + + expect(screen.getAllByText('Positioned')).toHaveLength(1); + }); + it('shows every message when called more than once in one update', () => { const notify = renderNotify(); diff --git a/src/custom/ErrorBoundary/ErrorBoundary.tsx b/src/custom/ErrorBoundary/ErrorBoundary.tsx index 0132e308f..63a3ae224 100644 --- a/src/custom/ErrorBoundary/ErrorBoundary.tsx +++ b/src/custom/ErrorBoundary/ErrorBoundary.tsx @@ -62,7 +62,7 @@ export function Fallback({ Error: - {(error as Error).message} + {error instanceof Error ? error.message : String(error)}
{showPackageInfo && ( diff --git a/src/custom/Helpers/Notification/notification-handler.ts b/src/custom/Helpers/Notification/notification-handler.ts index 29a3155cc..4910d7c83 100644 --- a/src/custom/Helpers/Notification/notification-handler.ts +++ b/src/custom/Helpers/Notification/notification-handler.ts @@ -13,6 +13,10 @@ export type NotificationHandlerOptions = { autoHideDuration?: number | null; persist?: boolean; preventDuplicate?: boolean; + anchorOrigin?: { + vertical: 'top' | 'bottom'; + horizontal: 'left' | 'center' | 'right'; + }; }; type NotificationHandler = (message: string, options?: NotificationHandlerOptions) => void;