fix(build): make index.d.ts and the runtime bundle export the same names

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/<domain>/` subtree, also add an explicit
-`export { Foo, type FooProps } from './<domain>/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
 

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",

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(
+    <SistentThemeProvider>
+      <ErrorBoundary>
+        <Thrower value={value} />
+      </ErrorBoundary>
+    </SistentThemeProvider>
+  );
+
+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}`);
+  });
+});

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['"]/);
-    }
-  );
 });

src/__testing__/ShareModalWireContract.test.tsx

@@ -95,6 +95,14 @@ const renderShareModal = (overrides: Partial<ShareModalProps> = {}) => {
 /** 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();

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<string>; all: Set<string> } => {
+  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<string>();
+  const all = new Set<string>();
+  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<string> => {
+  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<string> => {
+  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<string>, actual: Set<string>): 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<string>; all: Set<string> };
+    let commonJs: Set<string>;
+    let esModule: Set<string>;
+
+    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);
+      }
+    );
+  });
+});