|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * #5726 — no CLI production module may STATICALLY value-import a driver. |
| 5 | + * |
| 6 | + * The cost of one such import is not paid by the command that needs the driver. |
| 7 | + * oclif's `findCommand` walks the command table and `import()`s every command |
| 8 | + * module on **every** CLI invocation, so a broken import chain anywhere in that |
| 9 | + * table is charged to whatever command you actually ran. `schema-migrate.ts` is |
| 10 | + * shared by nine commands (`meta:resync`, `migrate`, and seven `migrate:*`), and |
| 11 | + * its one static `import { isInPlaceSchemaWork } from '@objectstack/driver-sql'` |
| 12 | + * meant that an unbuilt `packages/drivers/driver-sql/dist` made `os dev` print |
| 13 | + * nine `MODULE_NOT_FOUND` blocks (eighteen — `dev` forks a child) naming nine |
| 14 | + * commands the operator never invoked, and made all nine vanish from the command |
| 15 | + * table: `os migrate plan` answered `Command migrate:plan not found.` The real |
| 16 | + * cause was `pnpm build`, which nothing in that output said. |
| 17 | + * |
| 18 | + * These are source-level assertions on purpose. The defect lives in the shape of |
| 19 | + * the import graph, which is decided at authoring time and is invisible to any |
| 20 | + * test that merely calls the functions — every behavioural test in this package |
| 21 | + * runs in a workspace where the driver happens to be built. |
| 22 | + * |
| 23 | + * Scanning `src` rather than `dist` is the same choice: `dist` is what oclif |
| 24 | + * loads, but building it inside a unit test would be far slower than the thing |
| 25 | + * it guards, and `tsc` does not move imports between the two forms — a static |
| 26 | + * value import in `src` is a static import in `dist`, and an `await import()` |
| 27 | + * stays dynamic. |
| 28 | + */ |
| 29 | + |
| 30 | +import { describe, it, expect } from 'vitest'; |
| 31 | +import { readdirSync, readFileSync } from 'node:fs'; |
| 32 | +import { join } from 'node:path'; |
| 33 | +import { fileURLToPath } from 'node:url'; |
| 34 | + |
| 35 | +/** `packages/cli/src` — this file lives in `src/utils/`. */ |
| 36 | +const SRC_ROOT = fileURLToPath(new URL('..', import.meta.url)); |
| 37 | + |
| 38 | +/** Every production `.ts` under `packages/cli/src`, as `[relativePath, source]`. */ |
| 39 | +function productionSources(): Array<[string, string]> { |
| 40 | + return readdirSync(SRC_ROOT, { recursive: true, encoding: 'utf8' }) |
| 41 | + .filter((rel) => rel.endsWith('.ts') && !rel.endsWith('.d.ts')) |
| 42 | + .filter((rel) => !/\.(test|spec)\.ts$/.test(rel)) |
| 43 | + .map((rel) => [rel, readFileSync(join(SRC_ROOT, rel), 'utf8')] as [string, string]); |
| 44 | +} |
| 45 | + |
| 46 | +/** |
| 47 | + * Static `import … from '<specifier>'` statements, with the leading `type` |
| 48 | + * keyword captured when present. |
| 49 | + * |
| 50 | + * `[^;]*?` cannot cross a statement terminator, so a multi-line import clause is |
| 51 | + * matched whole while two adjacent statements can never be spliced together. |
| 52 | + */ |
| 53 | +const STATIC_IMPORT = /^[ \t]*import[ \t]+(?:(type)[ \t]+)?([^;]*?)[ \t]*from[ \t]*['"]([^'"]+)['"]/gm; |
| 54 | + |
| 55 | +/** Bare side-effect imports — `import '<specifier>';` — which also load the module. */ |
| 56 | +const SIDE_EFFECT_IMPORT = /^[ \t]*import[ \t]*['"]([^'"]+)['"]/gm; |
| 57 | + |
| 58 | +const DRIVER_PACKAGE = /^@objectstack\/driver-/; |
| 59 | + |
| 60 | +describe('#5726 — CLI command modules must not statically value-import a driver', () => { |
| 61 | + it('has no static value import of any @objectstack/driver-* package in production sources', () => { |
| 62 | + const offenders: string[] = []; |
| 63 | + |
| 64 | + for (const [rel, src] of productionSources()) { |
| 65 | + for (const m of src.matchAll(STATIC_IMPORT)) { |
| 66 | + const [, typeKeyword, clause, specifier] = m; |
| 67 | + if (!DRIVER_PACKAGE.test(specifier)) continue; |
| 68 | + // `import type { … } from` erases entirely — no runtime edge, no cost. |
| 69 | + if (typeKeyword) continue; |
| 70 | + // A clause of inline `type` specifiers still emits the module under |
| 71 | + // `verbatimModuleSyntax`. Flagged deliberately: the fix (hoisting the |
| 72 | + // keyword to `import type`) is trivial and always available, so there is |
| 73 | + // no reason to let the risky spelling through on a technicality. |
| 74 | + offenders.push(`${rel}: import ${clause.trim()} from '${specifier}'`); |
| 75 | + } |
| 76 | + for (const m of src.matchAll(SIDE_EFFECT_IMPORT)) { |
| 77 | + if (DRIVER_PACKAGE.test(m[1])) offenders.push(`${rel}: import '${m[1]}'`); |
| 78 | + } |
| 79 | + } |
| 80 | + |
| 81 | + expect( |
| 82 | + offenders, |
| 83 | + 'A static value import of a driver package makes EVERY command that reaches this module ' + |
| 84 | + 'fail oclif command discovery when the driver is not built, and prints one MODULE_NOT_FOUND ' + |
| 85 | + 'block per such command in front of whatever the operator actually ran (#5726). ' + |
| 86 | + 'Use `await import(\'@objectstack/driver-…\')` at the point of use, or `import type` when ' + |
| 87 | + 'only the types are needed.', |
| 88 | + ).toEqual([]); |
| 89 | + }); |
| 90 | + |
| 91 | + it('still reaches the driver for the in-place classifier, lazily — one definition, loaded later', () => { |
| 92 | + const src = readFileSync(join(SRC_ROOT, 'utils/schema-migrate.ts'), 'utf8'); |
| 93 | + |
| 94 | + // The point of the fix is NOT "stop depending on driver-sql". The |
| 95 | + // additive/in-place split is a fact about `PendingSchemaWorkKind`, declared |
| 96 | + // beside that union in the driver; re-deriving it here would let the CLI |
| 97 | + // disagree with the driver the day a kind is added — by listing a row |
| 98 | + // rewrite under a heading that promises the work is never data-losing |
| 99 | + // (#3954). Pin that the dependency survives, in its dynamic form. |
| 100 | + expect(src).toMatch(/await import\((['"])@objectstack\/driver-sql\1\)/); |
| 101 | + expect(src).toContain('isInPlaceSchemaWork'); |
| 102 | + }); |
| 103 | + |
| 104 | + it('awaits every call of the now-async pending-work renderers', () => { |
| 105 | + // `renderPendingSchemaWork` returns `Promise<void>`, so a dropped `await` is |
| 106 | + // not a type error — it is output that races the process exit. (The repo |
| 107 | + // already carries an eslint rule for exactly this shape on `formatOutput`.) |
| 108 | + const CALL = /(\bawait\s+|\bfunction\s+|\.)?\b(renderPendingSchemaWork|summarizePendingSchemaWork)\s*\(/g; |
| 109 | + const unawaited: string[] = []; |
| 110 | + |
| 111 | + for (const [rel, src] of productionSources()) { |
| 112 | + for (const m of src.matchAll(CALL)) { |
| 113 | + const prefix = m[1] ?? ''; |
| 114 | + if (/^function\s+$/.test(prefix)) continue; // the declaration itself |
| 115 | + if (/^await\s+$/.test(prefix)) continue; |
| 116 | + unawaited.push(`${rel}: ${m[0].trim()}`); |
| 117 | + } |
| 118 | + } |
| 119 | + |
| 120 | + expect(unawaited, 'These renderers became async in #5726 — await them.').toEqual([]); |
| 121 | + }); |
| 122 | +}); |
0 commit comments