diff --git a/AGENTS.md b/AGENTS.md index 217a5f976..e7b237ae2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -203,37 +203,28 @@ 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 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 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` +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..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", @@ -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..4879a19d3 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", @@ -136,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__/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__/RJSFFormWrapper.test.tsx b/src/__testing__/RJSFFormWrapper.test.tsx index c106c6574..441b98246 100644 --- a/src/__testing__/RJSFFormWrapper.test.tsx +++ b/src/__testing__/RJSFFormWrapper.test.tsx @@ -2,46 +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'); + expect((RJSFFormWrapper as unknown as { displayName: string }).displayName).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['"]/); - } - ); }); 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 new file mode 100644 index 000000000..e12327fce --- /dev/null +++ b/src/__testing__/declarationRuntimeExportParity.test.ts @@ -0,0 +1,170 @@ +/** + * @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 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`, + * 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 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'); + +/** 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', () => { + 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); + }); + + 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__/publishedTypeSurfaceDependencies.test.ts b/src/__testing__/publishedTypeSurfaceDependencies.test.ts index 5191646fd..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; @@ -120,6 +143,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 +161,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', () => { @@ -159,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 @@ -180,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 @@ -236,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. @@ -259,6 +327,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/__testing__/useNotificationHandler.test.tsx b/src/__testing__/useNotificationHandler.test.tsx new file mode 100644 index 000000000..b46ac2a06 --- /dev/null +++ b/src/__testing__/useNotificationHandler.test.tsx @@ -0,0 +1,51 @@ +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('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(); + + act(() => { + notify.current('First'); + notify.current('Second'); + }); + + expect(screen.getAllByText('First')).toHaveLength(1); + expect(screen.getAllByText('Second')).toHaveLength(1); + }); +}); 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..63a3ae224 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; @@ -50,7 +62,7 @@ export function Fallback({ Error: - {(error as Error).message} + {error instanceof Error ? error.message : String(error)}
{showPackageInfo && ( @@ -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..4910d7c83 100644 --- a/src/custom/Helpers/Notification/notification-handler.ts +++ b/src/custom/Helpers/Notification/notification-handler.ts @@ -1,27 +1,40 @@ -import { OptionsObject, useSnackbar } from 'notistack'; -import React from 'react'; +import { useSnackbar } from 'notistack'; -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; + anchorOrigin?: { + vertical: 'top' | 'bottom'; + horizontal: 'left' | 'center' | 'right'; + }; +}; + +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; 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/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/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 : ''; 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; +}