Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions .changeset/service-storage-test-tsc-program.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
"@objectstack/service-storage": patch
---

fix(service-storage): put the test layer in front of tsc, and repair what it was hiding (#15050)

`packages/services/service-storage` had **no `typecheck` script at all** — its
scripts were `build` and `test` — so no tsc program anywhere read this
package's test layer, and its errors were carried instead as a 51-error DEBT
entry in `scripts/check-type-check-coverage.mjs`. Gives it the #14062 /
#14181 "checked test zone" shape: a sibling `tsconfig.test.json` (module
semantics only — `esnext` / `bundler` / `lib: ES2022` — matching how vitest
actually executes these files; strictness inherited and untouched) plus a
`tsconfig.scripts.json` for `scripts/i18n-extract.config.ts` (the ninth
instance of #11351, previously excluded from that ledger only because this
package had no `typecheck` script to hang it on), both named by a new
`typecheck` script.

Measured before repair: 51 errors under BUILD semantics (`tsc --noEmit -p
tsconfig.json`, which already includes the tests — matching the DEBT entry's
recorded number exactly), 10 under the split. Unlike `service-cluster`
(#14181), this package's BUILD reading was *not* already clean, so both
programs needed genuine repair, not just the test-only split: 23 `TS2835`
(relative imports missing their `.js` extension, required under BUILD's
NodeNext resolution) were fixed by *adding* the extension — which resolves
correctly under both NodeNext and the split's bundler mode — and clearing
that also cleared all 15 `TS7006` "implicitly any" as a downstream cascade
from the same unresolved imports (the shape `@objectstack/core` reported at
98 → 4). The remaining 3 `TS2550` (`Array.prototype.at` needing `lib`
es2022) are rewritten to indexed access rather than widening the shared
BUILD `tsconfig.json`. The 8 code-tier errors (`TS2339` × 4 — a test
helper's object-spread dropped its `Record<string, unknown>` index
signature, fixed with an explicit return-shape annotation; `TS2347` × 4 — a
fake `ctx: any`'s `getService<T>(...)` calls converted to `getService(...)
as T`, the pattern one call site in the same file had already adopted for
exactly this reason) are genuine test-file fixes. Both readings now agree at
0/0 — the same result `service-cluster` reported, reached by a longer road.

The package's DEBT entry (51 errors) is **deleted**, not lowered — the
graduation this ratchet's invariant requires. No `test-typecheck-debt.json`
is added: residue is 0, so none is owed (#5286, maintainer-only to open).
`check:type-source-resolution` went red from onboarding the two new
programs (the documented onboarding-limb case): a registry entry is added
rather than `paths`, measured both ways — `paths` takes this package's test
layer from 0 errors to 306, all in other packages' source.

No runtime code changes: `src/**` excluding tests is byte-identical, so no
shipped behaviour moves. The `patch` level reflects the published
`package.json` gaining `typecheck` / `check:test-typecheck` scripts and a
`tsx` devDependency.
5 changes: 4 additions & 1 deletion packages/services/service-storage/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
},
"scripts": {
"build": "tsup --config ../../../tsup.config.ts && node ../../../scripts/check-dts-emitted.mjs",
"test": "vitest run"
"test": "vitest run",
"typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json && pnpm check:test-typecheck",
"check:test-typecheck": "tsx ../../../scripts/check-test-typecheck.mts --self-test && tsx ../../../scripts/check-test-typecheck.mts --package packages/services/service-storage --project tsconfig.test.json"
},
"dependencies": {
"@objectstack/core": "workspace:*",
Expand All @@ -45,6 +47,7 @@
"@objectstack/objectql": "workspace:*",
"@objectstack/driver-sql": "workspace:*",
"@types/node": "^26.2.0",
"tsx": "^4.23.12",
"typescript": "^6.0.3",
"vitest": "^4.1.10"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
import { LocalStorageAdapter } from './local-storage-adapter';
import { StorageMetadataStore } from './metadata-store';
import { registerStorageRoutes } from './storage-routes';
import { LocalStorageAdapter } from './local-storage-adapter.js';
import { StorageMetadataStore } from './metadata-store.js';
import { registerStorageRoutes } from './storage-routes.js';

const BASE = '/api/v1/storage';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,13 @@ function recordDispatch(scope: Record<string, unknown> = {}) {
async function driveInsert(engine: Engine, object: string, data: Record<string, unknown>, id: string) {
const ctx: any = { object, event: 'beforeInsert', input: { data }, dispatch: recordDispatch() };
await engine.trigger('beforeInsert', ctx);
const row = { ...(ctx.input.data as Record<string, unknown>), id };
// Explicitly typed rather than left to spread-inference: TS drops a spread
// source's index signature when it has no KNOWN properties, so an
// un-annotated `row` here infers as bare `{ id: string }` and every
// `row.<dataKey>` read below is an error — the shape IS
// `Record<string, unknown>` at runtime (the caller's `data` plus `id`), this
// just states it so the checker agrees.
const row: Record<string, unknown> & { id: string } = { ...(ctx.input.data as Record<string, unknown>), id };
(engine.tables[object] ??= []).push(row);
ctx.event = 'afterInsert';
ctx.result = row;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { InMemoryMetricsRegistry, SEMCONV } from '@objectstack/observability';
import { LocalStorageAdapter } from './local-storage-adapter';
import { LocalStorageAdapter } from './local-storage-adapter.js';

describe('LocalStorageAdapter instrumentation', () => {
let rootDir: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { describe, it, expect, afterEach } from 'vitest';
import { promises as fs } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { LocalStorageAdapter } from './local-storage-adapter';
import { LocalStorageAdapter } from './local-storage-adapter.js';
import type { IStorageService } from '@objectstack/spec/contracts';

describe('LocalStorageAdapter', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
*/

import { describe, it, expect } from 'vitest';
import { LocalStorageAdapter } from './local-storage-adapter';
import { S3StorageAdapter } from './s3-storage-adapter';
import { LocalStorageAdapter } from './local-storage-adapter.js';
import { S3StorageAdapter } from './s3-storage-adapter.js';

/** Every method name reachable on an instance, own + prototype chain. */
function reachableMethodNames(instance: object): string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ vi.mock('@aws-sdk/client-s3', () => {
// `vi.mock` is hoisted above every import in this file, so the lazy
// `await import('@aws-sdk/client-s3')` inside `S3StorageAdapter` resolves to the
// fake above no matter where these two sit.
import { LocalStorageAdapter } from './local-storage-adapter';
import { S3StorageAdapter } from './s3-storage-adapter';
import { LocalStorageAdapter } from './local-storage-adapter.js';
import { S3StorageAdapter } from './s3-storage-adapter.js';

// ---------------------------------------------------------------------------
// Backend harness
Expand Down Expand Up @@ -365,8 +365,8 @@ describe.each(BACKENDS)('$name adapter — list() conformance', ({ make }) => {
expect(page.keys).toHaveLength(400);
expect(page.nextCursor).toBeDefined();
}
expect(pages.at(-1)!.keys).toHaveLength(BULK_SET.length % 400);
expect(pages.at(-1)!.nextCursor).toBeUndefined();
expect(pages[pages.length - 1]!.keys).toHaveLength(BULK_SET.length % 400);
expect(pages[pages.length - 1]!.nextCursor).toBeUndefined();
}, 60_000);

it('fills one page past the backend\'s own page size (>1000 in a single call)', async () => {
Expand Down Expand Up @@ -434,7 +434,7 @@ describe.each(BACKENDS)('$name adapter — list() conformance', ({ make }) => {
it('issues a cursor the contract codec can read back', async () => {
const page = await backend.adapter.list!('bulk/', { limit: 10 });

expect(page.nextCursor).toBe(encodeStorageListCursor(page.items.at(-1)!.key));
expect(page.nextCursor).toBe(encodeStorageListCursor(page.items[page.items.length - 1]!.key));
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
*/

import { describe, it, expect, vi } from 'vitest';
import { registerStorageRoutes } from './storage-routes';
import { STORAGE_ROUTE_LEDGER } from './storage-route-ledger';
import { registerStorageRoutes } from './storage-routes.js';
import { STORAGE_ROUTE_LEDGER } from './storage-route-ledger.js';

/** Minimal IHttpServer mock that records registrations. */
function createMockServer() {
Expand Down
10 changes: 5 additions & 5 deletions packages/services/service-storage/src/storage-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
import { LocalStorageAdapter } from './local-storage-adapter';
import { StorageMetadataStore } from './metadata-store';
import { registerStorageRoutes } from './storage-routes';
import { LocalStorageAdapter } from './local-storage-adapter.js';
import { StorageMetadataStore } from './metadata-store.js';
import { registerStorageRoutes } from './storage-routes.js';
import { promises as fs } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
Expand Down Expand Up @@ -409,7 +409,7 @@ describe('Storage REST Routes', () => {
});

describe('attachments download gate (#2970 item 2)', () => {
const commit = async (s: StorageMetadataStore, rec: Partial<import('./metadata-store').FileRecord>) =>
const commit = async (s: StorageMetadataStore, rec: Partial<import('./metadata-store.js').FileRecord>) =>
s.createFile({
id: rec.id ?? 'f-dl',
key: rec.key ?? `attachments/${rec.id ?? 'f-dl'}.bin`,
Expand All @@ -421,7 +421,7 @@ describe('Storage REST Routes', () => {
...rec,
} as any);

function serverWith(verdict: import('./storage-routes').FileReadVerdict | 'skip', extra: any = {}) {
function serverWith(verdict: import('./storage-routes.js').FileReadVerdict | 'skip', extra: any = {}) {
const server = createMockHttpServer();
const s = new StorageMetadataStore(null);
const authorizeFileRead =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
OBSERVABILITY_METRICS_SERVICE,
SEMCONV,
} from '@objectstack/observability';
import { StorageServicePlugin } from './storage-service-plugin';
import { StorageServicePlugin } from './storage-service-plugin.js';

/**
* Mirror of `cache-service-plugin.metrics.test.ts` — verifies the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { describe, it, expect } from 'vitest';
import { promises as fs } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import type { IStorageService, StorageFileInfo } from '@objectstack/spec/contracts';
import { StorageServicePlugin } from './storage-service-plugin';
import { SwappableStorageService } from './swappable-storage-service';
import type { IStorageService } from '@objectstack/spec/contracts';
import { StorageServicePlugin } from './storage-service-plugin.js';
import { SwappableStorageService } from './swappable-storage-service.js';

/**
* Plugin-level integration test exercising the settings live-wire.
Expand Down Expand Up @@ -89,7 +89,10 @@ describe('StorageServicePlugin: settings live-wire', () => {
});
const ctx = makeCtx();
await plugin.init(ctx);
const svc = ctx.getService<IStorageService>('storage');
// Plain call with a cast, not `getService<T>(...)` — the fake ctx's
// getService is untyped, and each type-argument call adds a frozen-debt
// TS2347 to this package's shrink-only type-check ledger.
const svc = ctx.getService('storage') as IStorageService;
expect(svc).toBeInstanceOf(SwappableStorageService);
});

Expand Down Expand Up @@ -132,7 +135,7 @@ describe('StorageServicePlugin: settings live-wire', () => {

await plugin.init(ctx);
await plugin.start(ctx);
const proxy = ctx.getService<SwappableStorageService>('storage');
const proxy = ctx.getService('storage') as SwappableStorageService;
const innerBefore = proxy.getInner();

await ctx._flushReady();
Expand Down Expand Up @@ -160,7 +163,7 @@ describe('StorageServicePlugin: settings live-wire', () => {

await plugin.init(ctx);
await plugin.start(ctx);
const proxy = ctx.getService<SwappableStorageService>('storage');
const proxy = ctx.getService('storage') as SwappableStorageService;
const before = proxy.getInner();
await ctx._flushReady();
expect(proxy.getInner()).toBe(before);
Expand Down Expand Up @@ -414,7 +417,7 @@ describe('StorageServicePlugin: settings live-wire', () => {

await plugin.init(ctx);
await plugin.start(ctx);
const proxy = ctx.getService<SwappableStorageService>('storage');
const proxy = ctx.getService('storage') as SwappableStorageService;
const before = proxy.getInner();
await ctx._flushReady();
expect(proxy.getInner()).toBe(before); // no swap
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ import {
RawUploadResponseSchema,
} from '@objectstack/spec/api';
import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
import { LocalStorageAdapter } from './local-storage-adapter';
import { StorageMetadataStore } from './metadata-store';
import { registerStorageRoutes } from './storage-routes';
import { LocalStorageAdapter } from './local-storage-adapter.js';
import { StorageMetadataStore } from './metadata-store.js';
import { registerStorageRoutes } from './storage-routes.js';

const BASE = '/api/v1/storage';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
encodeStorageListCursor,
resolveStorageListLimit,
} from '@objectstack/spec/contracts';
import { SwappableStorageService } from './swappable-storage-service';
import { SwappableStorageService } from './swappable-storage-service.js';

class FakeAdapter implements IStorageService {
public name: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ describe('verifyFileReferences (ADR-0104 D3 wave 2 — R4 gate)', () => {
const many = Array.from({ length: 1200 }, (_, i) => ({ id: `p${i}`, image: `file_${i}` }));
const engine = fakeEngine({
product: many,
sys_file: many.map((r, i) => file(`file_${i}`, { object: 'product', recordId: `p${i}`, field: 'image' })),
sys_file: many.map((_r, i) => file(`file_${i}`, { object: 'product', recordId: `p${i}`, field: 'image' })),
});

const report = await verifyFileReferences(engine);
Expand Down
42 changes: 42 additions & 0 deletions packages/services/service-storage/tsconfig.scripts.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// The SCRIPTS-layer type-check program for this package (#11351, the ninth
// instance — see `scripts/check-type-check-coverage.mjs`'s
// `UNCHECKED_SOURCE_DEBT` docblock for why this package was deliberately left
// out of that ledger's eight: it had NO `typecheck` script at all, so
// SOURCES_COVERED's invariant — which only asks its question of a package
// that DECLARES one — never fired on it, and the directory below was covered
// instead by the (now-deleted) `DEBT['@objectstack/service-storage']` entry.
// #15050 gives this package a `typecheck` script, so this file completes the
// family the same PR that makes SOURCES_COVERED start asking here.
//
// `scripts/i18n-extract.config.ts` is the input to this package's i18n
// extraction: it composes the package's own objects and translation bundles
// into an `ObjectStackDefinition`. It is real source, and until this file
// existed no tsc program read a line of it -- `tsconfig.json` selects only
// `src`, so the package would have passed `check:type-check-coverage` as
// COVERED with the directory unread the moment `typecheck` existed.
//
// A SIBLING rather than a wider `include` on `tsconfig.json`, the
// distinction #5475 drew for `packages/spec` and #10756 for
// `packages/objectql/scripts`: that config emits, so widening it to reach
// `scripts/` would put the directory in front of the emit. This program
// emits nothing.
//
// STRICTNESS IS INHERITED and deliberately not relaxed: `strict`,
// `noUnusedLocals`, `noUnusedParameters`, `noImplicitReturns` and the rest
// come from the root config through `tsconfig.json`. The directory
// type-checks clean under them -- it enters with ZERO recorded debt, and
// there is no ledger here to record any in. Module semantics are inherited
// too (NodeNext): this config already spells its relative imports with `.js`
// extensions (#10868 annotated it), so nothing here needs `packages/spec`'s
// bundler resolution or `allowImportingTsExtensions`.
{
"extends": "./tsconfig.json",
"compilerOptions": {
"noEmit": true,
// `.` rather than the inherited `src`, because the file this program
// checks is the one outside `src`. Safe precisely because nothing is
// emitted from here -- see the header.
"rootDir": "."
},
"include": ["scripts/**/*"]
}
Loading
Loading