Skip to content

Commit 82faea0

Browse files
os-trumpclaude
andauthored
fix(rest): state the exceljs Buffer-shim assertion once, in a typed test loader (#13425)
`exceljs@4.4.0`'s `index.d.ts` opens with `declare interface Buffer extends ArrayBuffer { }`. That file has 106 top-level exports, so it is a module and this `Buffer` is module-local: it shadows Node's global `Buffer` inside every exceljs signature, including `load(buffer: Buffer, ...)` at `index.d.ts:1490`. A Node `Buffer` is a `Uint8Array`, so no Node `Buffer` value can satisfy that parameter. The defect is in the published declaration, not at any call site. `packages/rest` paid it at 6 anonymous `as any` sites and left a 7th as a ledgered `TS2345`. This adds `src/xlsx-test-loader.ts` — one `loadXlsxWorkbook` that performs the assertion once, against the parameter type read off the dependency's own signature (`Parameters<Xlsx['load']>[0]`) rather than spelled by hand — and migrates all 7 test sites onto it. Runtime is untouched: the same Node `Buffer` reaches exceljs, which has always accepted it. `src/import-prepare.ts:143` is production source and is deliberately unchanged; it types its dynamic exceljs import as `any` so CSV/JSON imports do not pay for the dependency, and a test-layer helper cannot serve it. Ledger: `src/rest.test.ts` 3 -> 2, regenerated with `pnpm --filter @objectstack/rest gen:test-typecheck-debt`. The two that remain are the `IHttpRequest` pair; the authored `_note` is updated to point at the card that holds them. Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k Co-authored-by: Claude <noreply@anthropic.com>
1 parent 34e8099 commit 82faea0

5 files changed

Lines changed: 100 additions & 19 deletions

File tree

packages/rest/src/export-business-timezone.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@
3131
*/
3232

3333
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
34-
import ExcelJS from 'exceljs';
3534
import { ObjectQL } from '@objectstack/objectql';
3635
import { SqlDriver } from '@objectstack/driver-sql';
3736
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
3837
import { RestServer } from './rest-server.js';
3938
import { formatCellValue, formatRowCells, formatRowForJson } from './export-format.js';
4039
import type { ExportFieldMeta } from './export-format.js';
40+
import { loadXlsxWorkbook } from './xlsx-test-loader.js';
4141

4242
// The instant at the heart of the report: 2026-08-01 06:00 in +08 is
4343
// 2026-07-31 22:00 in UTC — a different day, month and quarter-of-year.
@@ -238,8 +238,7 @@ async function xlsxRow(timezone?: string): Promise<string[]> {
238238
const route = await boot(timezone);
239239
const { res, getBuffer } = makeBinRes();
240240
await route.handler({ params: { object: 'shift' }, query: { format: 'xlsx' } } as any, res);
241-
const wb = new ExcelJS.Workbook();
242-
await wb.xlsx.load(getBuffer() as any);
241+
const wb = await loadXlsxWorkbook(getBuffer());
243242
return (wb.worksheets[0].getRow(2).values as any[]).slice(1).map((v) => String(v));
244243
}
245244

packages/rest/src/export-integration.test.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@
3333
*/
3434

3535
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
36-
import ExcelJS from 'exceljs';
3736
import { ObjectQL } from '@objectstack/objectql';
3837
import { SqlDriver } from '@objectstack/driver-sql';
3938
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
4039
import { maskFieldValue } from '@objectstack/plugin-security';
4140
import { RestServer } from './rest-server';
41+
import { loadXlsxWorkbook } from './xlsx-test-loader.js';
4242

4343
// ---------------------------------------------------------------------------
4444
// The real backend: better-sqlite3 `:memory:`, constructed the canonical way
@@ -211,8 +211,7 @@ describe('export route — real engine + protocol integration', () => {
211211
expect(headers['Content-Type']).toBe(
212212
'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
213213
);
214-
const wb = new ExcelJS.Workbook();
215-
await wb.xlsx.load(getBuffer() as any);
214+
const wb = await loadXlsxWorkbook(getBuffer());
216215
const ws = wb.worksheets[0];
217216
const header = (ws.getRow(1).values as any[]).slice(1).map((v) => String(v));
218217
expect(header).toEqual(['ID', '标题', '完成', '优先级', '截止', '负责人']);
@@ -227,8 +226,7 @@ describe('export route — real engine + protocol integration', () => {
227226
// Default limit (10000) is within the style cap, so colours are applied.
228227
expect(headers['X-Export-Styles']).toBe('applied');
229228

230-
const wb = new ExcelJS.Workbook();
231-
await wb.xlsx.load(getBuffer() as any);
229+
const wb = await loadXlsxWorkbook(getBuffer());
232230
const ws = wb.worksheets[0];
233231
// priority is column 4 (ID, 标题, 完成, 优先级, ...).
234232
const highCell = ws.getRow(2).getCell(4); // '高' → #e11d48
@@ -247,8 +245,7 @@ describe('export route — real engine + protocol integration', () => {
247245
);
248246

249247
expect(headers['X-Export-Styles']).toBe('dropped');
250-
const wb = new ExcelJS.Workbook();
251-
await wb.xlsx.load(getBuffer() as any);
248+
const wb = await loadXlsxWorkbook(getBuffer());
252249
const ws = wb.worksheets[0];
253250
// Data is intact...
254251
const r1 = (ws.getRow(2).values as any[]).slice(1).map((v) => String(v));
@@ -528,8 +525,7 @@ describe('export route — FLS column projection via getReadableFields (#3547)',
528525
});
529526
const { res, getBuffer } = makeBinRes();
530527
await route.handler({ params: { object: 'task' }, query: { format: 'xlsx' } } as any, res);
531-
const wb = new ExcelJS.Workbook();
532-
await wb.xlsx.load(getBuffer() as any);
528+
const wb = await loadXlsxWorkbook(getBuffer());
533529
const ws = wb.worksheets[0];
534530
expect((ws.getRow(1).values as any[]).slice(1)).toEqual(['ID', '完成']);
535531
expect(ws.rowCount).toBe(1); // header only
@@ -610,8 +606,7 @@ describe('export route — search', () => {
610606
{ params: { object: 'task' }, query: { format: 'xlsx', search: '代码' } } as any,
611607
res,
612608
);
613-
const wb = new ExcelJS.Workbook();
614-
await wb.xlsx.load(getBuffer() as any);
609+
const wb = await loadXlsxWorkbook(getBuffer());
615610
expect(wb.worksheets[0].rowCount).toBe(2); // header + one match
616611
});
617612
});

packages/rest/src/rest.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
4-
import ExcelJS from 'exceljs';
54
import { RouteManager } from './route-manager';
65
import { RestServer, mapDataError } from './rest-server';
76
import { createRestApiPlugin } from './rest-api-plugin';
87
import type { RestApiPluginConfig } from './rest-api-plugin';
8+
import { loadXlsxWorkbook } from './xlsx-test-loader.js';
99

1010
// ---------------------------------------------------------------------------
1111
// Mocks & Helpers
@@ -1263,8 +1263,7 @@ describe('RestServer', () => {
12631263
// xlsx is a zip — verify the PK signature, then round-trip the content.
12641264
expect(buf.subarray(0, 2).toString('latin1')).toBe('PK');
12651265

1266-
const wb = new ExcelJS.Workbook();
1267-
await wb.xlsx.load(buf);
1266+
const wb = await loadXlsxWorkbook(buf);
12681267
const ws = wb.getWorksheet('Export');
12691268
expect(ws).toBeDefined();
12701269
// row.values is 1-indexed (values[0] is empty).
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The one place this package asserts around `exceljs`'s broken `load` signature
5+
* (#13378). Test layer only — nothing in `src/index.ts` reaches it, so tsup
6+
* (entry: `src/index.ts`) never emits it into `dist` and it is not published.
7+
*
8+
* ## Why the assertion below is unavoidable, in the dependency's own bytes
9+
*
10+
* `exceljs@4.4.0/index.d.ts` opens, at line 1, with:
11+
*
12+
* ```ts
13+
* declare interface Buffer extends ArrayBuffer { }
14+
* ```
15+
*
16+
* That file carries 106 top-level `export` declarations, so it **is a module** —
17+
* which makes this `Buffer` module-local, and it therefore SHADOWS Node's global
18+
* `Buffer` inside every exceljs signature. The one that matters here is the
19+
* `Xlsx.load` at `index.d.ts:1490`:
20+
*
21+
* ```ts
22+
* load(buffer: Buffer, options?: Partial<XlsxReadOptions>): Promise<Workbook>;
23+
* ```
24+
*
25+
* ⇒ `Workbook.xlsx.load` does not ask for a Node `Buffer`. It asks for something
26+
* structurally identical to `ArrayBuffer`. A Node `Buffer` is a `Uint8Array`, so
27+
* it is not assignable, and tsc says so precisely:
28+
*
29+
* ```
30+
* src/rest.test.ts(1267,26): error TS2345: Argument of type 'Buffer<ArrayBuffer>' is not assignable to parameter of type 'Buffer'.
31+
* The types of 'slice(...)[Symbol.toStringTag]' are incompatible between these types.
32+
* Type '"Uint8Array"' is not assignable to type '"ArrayBuffer"'.
33+
* ```
34+
*
35+
* ⭐ There is NO Node `Buffer` value that satisfies that parameter. The defect is
36+
* in the published declaration, not at any call site — so this is not laziness,
37+
* and no amount of care at a call site can remove it. What a call site CAN do is
38+
* not restate it: before #13378 the package paid this at 6 anonymous `as any`s
39+
* and left a 7th site as a ledgered `TS2345`. Now it is stated once, here.
40+
*
41+
* ## Why option C (upgrade) is not the answer — measured 2026-08-30
42+
*
43+
* `exceljs` `dist-tags.latest` IS 4.4.0 (published 2023-10-19). The only publish
44+
* after it in the package's whole 166-version history is `4.4.1-prerelease.0`
45+
* (2024-12-20), and its `index.d.ts` carries the identical declaration at the
46+
* identical lines: shim at line 1, 106 exports, `load(buffer: Buffer, …)` at
47+
* 1490. The registry's `time.modified` is that same date. There is no later line
48+
* to pin to. (4.3.0 ships the shim too, so this is not a 4.4.0 regression that a
49+
* bump could undo.)
50+
*
51+
* ## Why not a declaration override (option B)
52+
*
53+
* The `Buffer` above is module-local, so it cannot be reached by interface
54+
* augmentation from outside; an override would have to redeclare the module,
55+
* replacing the package's entire typing surface. Far more surface than the one
56+
* assertion it would remove.
57+
*
58+
* ## Runtime is untouched
59+
*
60+
* exceljs's `load` has always accepted the Node `Buffer` these tests hand it —
61+
* that is what every one of those 6 `as any` sites was doing, and passing. This
62+
* helper changes only what tsc is told; it does not convert, copy or reshape the
63+
* bytes.
64+
*/
65+
66+
import ExcelJS from 'exceljs';
67+
import type { Workbook, Xlsx } from 'exceljs';
68+
69+
/**
70+
* The parameter type `Xlsx.load` actually declares, read off the dependency's
71+
* own signature instead of spelled by hand. Naming it this way means that if
72+
* exceljs ever drops the shim, this alias resolves to Node's `Buffer` and the
73+
* assertion below quietly becomes a no-op rather than a lie.
74+
*/
75+
type XlsxLoadInput = Parameters<Xlsx['load']>[0];
76+
77+
/**
78+
* Load .xlsx bytes into a fresh {@link Workbook}.
79+
*
80+
* The single site in this package where the exceljs declaration defect above is
81+
* asserted away. Callers pass the Node `Buffer` their fixture produced and get
82+
* back a workbook; no call site needs to know about any of this.
83+
*/
84+
export async function loadXlsxWorkbook(bytes: Buffer): Promise<Workbook> {
85+
const wb = new ExcelJS.Workbook();
86+
await wb.xlsx.load(bytes as unknown as XlsxLoadInput);
87+
return wb;
88+
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
{
22
"_comment": "Per-file tsc error debt of the @objectstack/rest TEST layer (#5286). `tsconfig.test.json` compiles `src/**/*.test.ts` — which `tsconfig.json` excludes and therefore no gate ever read — and every file below still carries errors from before that gate existed. THIS FIELD IS GENERATED: every regeneration rewrites it from scripts/check-test-typecheck.mts, and the EXACT ratchet below requires a regeneration on every repair — so an edit made here is gone by the next one. Anything true of THIS package goes in the sibling `_note` field, which is authored, is preserved verbatim, and is never written by the generator (#12624). This comment states NO cause for the errors, deliberately: the classes differ per package and per file, they move as the debt is paid down, and a cause written here is rewritten verbatim into every ledger by every regeneration — so it outlives its own repair and cannot be corrected in the file where it is read. Measure instead, before repairing anything: `tsc --noEmit --pretty false -p tsconfig.test.json` in the package prints the real classes with their TS codes. EXACT ratchet, judged by re-running tsc: a file that gains errors is red, a file that loses them is red until its number is re-recorded, a file that reaches zero is red until its entry is deleted, and a file NOT listed here may have no errors at all. Regenerate with: pnpm --filter @objectstack/rest gen:test-typecheck-debt",
3-
"_note": "Everything still recorded here is held by its own card, and none of it is an annotation repair. #13377 holds the request literals this package builds by hand against IHttpRequest, which omit members that interface requires. #13378 holds the exceljs call whose parameter type resolves to that package's own module-local Buffer declaration rather than Node's. Repairing either one here would mean changing a fixture's data, or adding an assertion this package's paydown rules out, so these entries shrink when those cards land and not before.",
3+
"_note": "Everything still recorded here is held by its own card, and none of it is an annotation repair. #13377 holds every entry that remains: the request literals this package builds by hand against IHttpRequest, which omit members that interface requires. Repairing them here would mean changing a fixture's data, so these entries shrink when that card lands and not before. The exceljs call that #13378 held is no longer in this ledger: that dependency declares its own module-local Buffer, which shadows Node's inside every exceljs signature, so no Node Buffer can be passed to Workbook.xlsx.load — the assertion that costs is now stated once, in src/xlsx-test-loader.ts, and every xlsx-reading test in this package goes through it.",
44
"entries": {
55
"src/meta-public-book-grant.test.ts": 1,
66
"src/rest-batch-size-cap.test.ts": 1,
7-
"src/rest.test.ts": 3
7+
"src/rest.test.ts": 2
88
}
99
}

0 commit comments

Comments
 (0)