mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
2180dd8e6a
We've had lots of type errors in our tests, scripts and other non-core
code despite all CI being green. This PR fixes that.
This PR also removes some redundant features like `@ts-check` inline
directives which have been replaced by proper ts config files.
<details>
## AI Summary
Audit of every CI workflow against every package's `package.json` and
`tsconfig` surfaced several gaps where CI did partial coverage. This PR
closes them and fixes every pre-existing type error the new gates
surfaced.
## Gaps closed
| # | Gap | Fix |
|---|---|---|
| 1 | `tsc --build` only covers `src/**`; test files (`test/**`,
`tests/**`) never typecheck. Vitest transforms tests with esbuild —
strips types instead of checking them. | New `tsconfig.test.json` per
test-bearing package (`composite: false`, `noEmit`, `rootDir: "."`,
includes `src/**` + `test/**` (or `tests/**`) + `vitest.config.ts`). New
`typecheck:tests` npm script per package. New CI step `npm run
typecheck:tests --workspaces --if-present`. |
| 2 | Build/publish scripts (`scripts/*.{cjs,mjs}`,
`packages/argent/scripts/*.cjs`, `packages/skills/scripts/install.js`) —
JS, never typechecked. `sync-readme.cjs` runs in `prepack`;
`postinstall.cjs` runs on every install; `bundle-tools.cjs` is
build-critical. A typo only surfaces at runtime. | Root
`tsconfig.scripts.json` with `allowJs` + `checkJs` + `noImplicitReturns`
covers all script paths. Root `typecheck:scripts` npm script. New CI
step. Per-file `// @ts-check` comments removed — tsconfig drives
inclusion. |
| 3 | `argent-cli` had no `typecheck:tests` script. When tests get added
there they'd silently bypass the gate. | Added `tsconfig.test.json` +
`typecheck:tests` script. `--if-present` would have skipped it anyway,
so this is future-proofing. |
| 4 | Local `prettier --check .` walked into the `argent-private`
submodule and reported violations there. CI didn't see this because
`actions/checkout@v4` doesn't init submodules — so local `prettier
--check` did not match CI behavior. | New `.prettierignore` excludes the
submodule plus `dist/`, `node_modules/`, lock files, `tsbuildinfo`.
No-op in CI; aligns local with remote. |
## Test type errors fixed
These were already present. The `npm test` passed because esbuild
stripped the offending types:
- **argent-installer**: widen `scope` literal type for
dead-code-elimination guard; non-null `addAllowlist!`/`removeAllowlist!`
(optional methods on adapter).
- **argent-mcp**: add `.js` extensions for NodeNext module resolution.
- **registry**: broaden `StaticBlueprintResult` blueprint api from `{
id; deps? }` to `Record<string, unknown>` so existing tests can pass
arbitrary api shapes.
- **tool-server**:
- non-null `zodSchema!` on tool defs that always carry one
(`launchAppTool`, `restartAppTool`, etc.)
- cast sentinel `"ignored"` strings to `DeviceInfo` in factory-rejection
tests
- match the 4-arg `dispatchByPlatform<IosServices, AndroidServices,
Params, Result>` signature (tests still passed the old 3-arg shape)
- `assertFlowRunResult` type guard for the `FlowRunResult |
FlowPrerequisiteNotice` union before `.steps` access
- swap broken `typeof import("supertest").default` for static `import
supertest` (supertest is `export = supertest`)
- fill in the 4 fields missing from a `NativeProfilerSessionApi` mock
(`xctraceProcess`, `recordingTimedOut`, `recordingExitedUnexpectedly`,
`lastExitInfo`)
- cast a vitest `Mock` to a plain function at one direct call site where
the `Procedure | Constructable` union loses callability
- fix `update-checker.test.ts` reading `../../package.json` (off-by-one
— went past tool-server; was relying on a Vite resolver quirk)
- replace one cross-package `../../../registry/src/index` import with
`@argent/registry` so registry's private `services` field isn't compared
between src and dist declarations
All fixes are type-level. Full `npm test --workspaces --if-present`
still reports **991 passed across 87 test files**.
</details>
291 lines
9.6 KiB
TypeScript
291 lines
9.6 KiB
TypeScript
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
import { BOOTSTRAP_DEVTOOLS_BACKEND_SCRIPT } from "../../src/utils/react-profiler/scripts";
|
|
|
|
/**
|
|
* Regression tests for the self-bootstrap path that lets react-profiler-start
|
|
* recover when no external React DevTools client is connected. The script
|
|
* is a self-contained IIFE string evaluated against a mock global hook and
|
|
* Metro module registry — covers each failure mode that maps to a distinct
|
|
* user-facing error in `react-profiler-start.ts`.
|
|
*
|
|
* See `argent-react-profiler-bug.md` (Defect 2) for the motivating bug.
|
|
*/
|
|
|
|
interface BootstrapResult {
|
|
ok: boolean;
|
|
reason: string;
|
|
renderersCount?: number;
|
|
rendererInterfacesCount?: number;
|
|
message?: string;
|
|
}
|
|
|
|
function evalIIFE(script: string): string {
|
|
return (0, eval)(script) as string;
|
|
}
|
|
|
|
interface MockHookOpts {
|
|
noHook?: boolean;
|
|
renderers?: Map<number, unknown>;
|
|
rendererInterfaces?: Map<number, unknown>;
|
|
}
|
|
|
|
interface MockRegistryEntry {
|
|
verboseName?: string;
|
|
module?: Record<string, unknown>;
|
|
}
|
|
|
|
interface ScenarioOpts {
|
|
hook?: MockHookOpts;
|
|
metro?: "map" | "array" | "missing" | "throws";
|
|
registry?: Record<number, MockRegistryEntry>;
|
|
requireThrows?: number[];
|
|
}
|
|
|
|
function runScenario(opts: ScenarioOpts): BootstrapResult {
|
|
const g = globalThis as Record<string, unknown>;
|
|
const originalHook = g.__REACT_DEVTOOLS_GLOBAL_HOOK__;
|
|
const originalR = g.__r;
|
|
|
|
if (!opts.hook?.noHook) {
|
|
g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = {
|
|
renderers: opts.hook?.renderers ?? new Map(),
|
|
rendererInterfaces: opts.hook?.rendererInterfaces ?? new Map(),
|
|
};
|
|
} else {
|
|
g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = undefined;
|
|
}
|
|
|
|
if (opts.metro === "missing") {
|
|
g.__r = undefined;
|
|
} else if (opts.metro === "throws") {
|
|
const fn = ((id: number) => {
|
|
if (opts.requireThrows?.includes(id)) throw new Error(`require-${id}-throws`);
|
|
const entry = opts.registry?.[id];
|
|
return entry?.module ?? null;
|
|
}) as ((id: number) => unknown) & { getModules: () => unknown };
|
|
fn.getModules = () => {
|
|
throw new Error("getModules-throws");
|
|
};
|
|
g.__r = fn;
|
|
} else {
|
|
const fn = ((id: number) => {
|
|
if (opts.requireThrows?.includes(id)) throw new Error(`require-${id}-throws`);
|
|
const entry = opts.registry?.[id];
|
|
return entry?.module ?? null;
|
|
}) as ((id: number) => unknown) & { getModules: () => unknown };
|
|
fn.getModules = () => {
|
|
const reg = opts.registry ?? {};
|
|
if (opts.metro === "array") {
|
|
const arr: Array<{ verboseName?: string }> = [];
|
|
for (const [id, entry] of Object.entries(reg)) {
|
|
arr[Number(id)] = { verboseName: entry.verboseName };
|
|
}
|
|
return arr;
|
|
}
|
|
// Default: Map<id, meta>
|
|
const m = new Map<number, { verboseName?: string }>();
|
|
for (const [id, entry] of Object.entries(reg)) {
|
|
m.set(Number(id), { verboseName: entry.verboseName });
|
|
}
|
|
return m;
|
|
};
|
|
g.__r = fn;
|
|
}
|
|
|
|
try {
|
|
const json = evalIIFE(BOOTSTRAP_DEVTOOLS_BACKEND_SCRIPT);
|
|
return JSON.parse(json) as BootstrapResult;
|
|
} finally {
|
|
g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = originalHook;
|
|
g.__r = originalR;
|
|
}
|
|
}
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
describe("BOOTSTRAP_DEVTOOLS_BACKEND_SCRIPT", () => {
|
|
it("returns no-hook when __REACT_DEVTOOLS_GLOBAL_HOOK__ is missing", () => {
|
|
const result = runScenario({ hook: { noHook: true } });
|
|
expect(result).toEqual({ ok: false, reason: "no-hook" });
|
|
});
|
|
|
|
it("returns already-attached when rendererInterfaces is non-empty", () => {
|
|
const ri = new Map<number, unknown>([
|
|
[1, { id: 1 }],
|
|
[2, { id: 2 }],
|
|
]);
|
|
const renderers = new Map<number, unknown>([
|
|
[1, {}],
|
|
[2, {}],
|
|
]);
|
|
const result = runScenario({ hook: { renderers, rendererInterfaces: ri } });
|
|
expect(result).toMatchObject({
|
|
ok: true,
|
|
reason: "already-attached",
|
|
renderersCount: 2,
|
|
rendererInterfacesCount: 2,
|
|
});
|
|
});
|
|
|
|
it("returns no-renderers when React hasn't injected any renderer yet", () => {
|
|
const result = runScenario({
|
|
hook: { renderers: new Map(), rendererInterfaces: new Map() },
|
|
});
|
|
expect(result).toMatchObject({
|
|
ok: false,
|
|
reason: "no-renderers",
|
|
renderersCount: 0,
|
|
rendererInterfacesCount: 0,
|
|
});
|
|
});
|
|
|
|
it("returns no-metro-modules when __r.getModules is unavailable", () => {
|
|
const result = runScenario({
|
|
hook: { renderers: new Map([[1, {}]]), rendererInterfaces: new Map() },
|
|
metro: "missing",
|
|
});
|
|
expect(result).toMatchObject({ ok: false, reason: "no-metro-modules" });
|
|
});
|
|
|
|
it("returns no-rdt-module when react-devtools-core is not in the bundle", () => {
|
|
const result = runScenario({
|
|
hook: { renderers: new Map([[1, {}]]), rendererInterfaces: new Map() },
|
|
registry: {
|
|
100: { verboseName: "node_modules/react-native/Libraries/Foo.js" },
|
|
},
|
|
});
|
|
expect(result).toMatchObject({ ok: false, reason: "no-rdt-module" });
|
|
});
|
|
|
|
it("returns unsupported-rdt-version when rdt-core lacks connectWithCustomMessagingProtocol", () => {
|
|
const result = runScenario({
|
|
hook: { renderers: new Map([[1, {}]]), rendererInterfaces: new Map() },
|
|
registry: {
|
|
205: {
|
|
verboseName: "node_modules/react-devtools-core/dist/backend.js",
|
|
// Only connectToDevTools — pre-5.1 API surface
|
|
module: { connectToDevTools: () => undefined },
|
|
},
|
|
},
|
|
});
|
|
expect(result).toMatchObject({ ok: false, reason: "unsupported-rdt-version" });
|
|
});
|
|
|
|
it("returns bootstrapped after a successful connectWithCustomMessagingProtocol call (Map registry)", () => {
|
|
const rendererInterfaces = new Map<number, unknown>();
|
|
const renderers = new Map<number, unknown>([
|
|
[1, {}],
|
|
[2, {}],
|
|
]);
|
|
const connect = vi.fn((_options?: Record<string, unknown>) => {
|
|
// Simulate initBackend populating rendererInterfaces
|
|
rendererInterfaces.set(1, { id: 1 });
|
|
rendererInterfaces.set(2, { id: 2 });
|
|
});
|
|
const result = runScenario({
|
|
hook: { renderers, rendererInterfaces },
|
|
registry: {
|
|
205: {
|
|
verboseName: "node_modules/react-devtools-core/dist/backend.js",
|
|
module: { connectWithCustomMessagingProtocol: connect },
|
|
},
|
|
},
|
|
});
|
|
expect(result).toMatchObject({
|
|
ok: true,
|
|
reason: "bootstrapped",
|
|
renderersCount: 2,
|
|
rendererInterfacesCount: 2,
|
|
});
|
|
expect(connect).toHaveBeenCalledOnce();
|
|
const arg = connect.mock.calls[0][0] as Record<string, unknown>;
|
|
expect(typeof arg.onSubscribe).toBe("function");
|
|
expect(typeof arg.onUnsubscribe).toBe("function");
|
|
expect(typeof arg.onMessage).toBe("function");
|
|
});
|
|
|
|
it("returns bootstrapped with array-style Metro registry", () => {
|
|
const rendererInterfaces = new Map<number, unknown>();
|
|
const renderers = new Map<number, unknown>([[1, {}]]);
|
|
const connect = vi.fn(() => {
|
|
rendererInterfaces.set(1, { id: 1 });
|
|
});
|
|
const result = runScenario({
|
|
hook: { renderers, rendererInterfaces },
|
|
metro: "array",
|
|
registry: {
|
|
17: {
|
|
verboseName: "node_modules/react-devtools-core/dist/backend.js",
|
|
module: { connectWithCustomMessagingProtocol: connect },
|
|
},
|
|
},
|
|
});
|
|
expect(result).toMatchObject({ ok: true, reason: "bootstrapped" });
|
|
expect(connect).toHaveBeenCalledOnce();
|
|
});
|
|
|
|
it("returns metro-scan-error when getModules throws", () => {
|
|
const result = runScenario({
|
|
hook: { renderers: new Map([[1, {}]]), rendererInterfaces: new Map() },
|
|
metro: "throws",
|
|
});
|
|
expect(result.ok).toBe(false);
|
|
expect(result.reason).toBe("metro-scan-error");
|
|
expect(result.message).toContain("getModules-throws");
|
|
});
|
|
|
|
it("returns bootstrap-threw when connectWithCustomMessagingProtocol raises", () => {
|
|
const result = runScenario({
|
|
hook: { renderers: new Map([[1, {}]]), rendererInterfaces: new Map() },
|
|
registry: {
|
|
205: {
|
|
verboseName: "node_modules/react-devtools-core/dist/backend.js",
|
|
module: {
|
|
connectWithCustomMessagingProtocol: () => {
|
|
throw new Error("bridge-init-failed");
|
|
},
|
|
},
|
|
},
|
|
},
|
|
});
|
|
expect(result.ok).toBe(false);
|
|
expect(result.reason).toBe("bootstrap-threw");
|
|
expect(result.message).toContain("bridge-init-failed");
|
|
});
|
|
|
|
it("returns bootstrap-no-effect when connectWith… returns without populating interfaces", () => {
|
|
const result = runScenario({
|
|
hook: { renderers: new Map([[1, {}]]), rendererInterfaces: new Map() },
|
|
registry: {
|
|
205: {
|
|
verboseName: "node_modules/react-devtools-core/dist/backend.js",
|
|
// No-op: does not populate rendererInterfaces (simulates a broken backend)
|
|
module: { connectWithCustomMessagingProtocol: () => undefined },
|
|
},
|
|
},
|
|
});
|
|
expect(result).toMatchObject({ ok: false, reason: "bootstrap-no-effect" });
|
|
});
|
|
|
|
it("survives require() throwing for non-matching modules during scan", () => {
|
|
const rendererInterfaces = new Map<number, unknown>();
|
|
const connect = vi.fn(() => {
|
|
rendererInterfaces.set(1, { id: 1 });
|
|
});
|
|
const result = runScenario({
|
|
hook: { renderers: new Map([[1, {}]]), rendererInterfaces },
|
|
registry: {
|
|
100: { verboseName: "node_modules/some-other/lib/index.js" },
|
|
205: {
|
|
verboseName: "node_modules/react-devtools-core/dist/backend.js",
|
|
module: { connectWithCustomMessagingProtocol: connect },
|
|
},
|
|
},
|
|
requireThrows: [100],
|
|
});
|
|
expect(result).toMatchObject({ ok: true, reason: "bootstrapped" });
|
|
});
|
|
});
|