XLS-352: pin every zip entry's mtime via an optional zipEntryDate option - #2
XLS-352: pin every zip entry's mtime via an optional zipEntryDate option#2senoff wants to merge 1 commit into
Conversation
Zip-entry mtime (`date`) is a per-entry archiver/JSZip option that defaults
to `new Date()` at append time; a module-level `zip: {date}` option is a
no-op (never forwarded), so output was non-deterministic at DOS-format
2-second granularity (XLS-350's "coin flip" determinism issue).
Threads a new `zipEntryDate?: Date` option through both writer paths:
- streaming `stream.xlsx.WorkbookWriter`, via a new `_append(data, options)`
choke point every internal `zip.append` call now goes through.
- buffered `Workbook.xlsx.writeBuffer`/`writeFile`, via a new
`ZipWriter._pinEntryDates()` called at `finalize()` (right before
`zip.generateAsync`), which sweeps ALL `this.zip.files` — including
JSZip's auto-created folder entries (`xl/`, `xl/worksheets/`, ...) that
never pass through `.append` and would otherwise keep the wall clock.
Left undefined (the default), every entry keeps the wall clock exactly as
before — existing callers see no change.
This is the durable, in-fork replacement for the server-side
`PinnedWorkbookWriter` subclass, which reaches past the published exceljs
surface to monkey-patch `zip.append` from outside the base constructor.
Once this lands and the server's @protobi/exceljs pin is bumped, that
subclass becomes deletable in favor of passing `zipEntryDate` directly.
New spec (spec/integration/issues/issue-xls352-zip-entry-mtime-pin.spec.js)
covers both writer paths with 3 arms each: byte-identity across a real
>2s delay when pinned, per-entry mtime stamping, and a positive control
proving the option is causally responsible (bytes differ / wall clock
entries when omitted). 6/6 passing; full unit (886/1 pending) and
integration (208) suites pass with no regressions.
Also fixes two pre-existing lint-config-drift blockers this diff's
lint-staged run surfaced on the touched files (unrelated to the
zipEntryDate change itself, and out of scope for the standing
fix/prettier-eslint-config-drift branch which reformats the whole repo):
- workbook-writer.js: `require('../../xlsx/xml/theme1.js')` violated
import/extensions (real, pre-existing eslint error independent of
prettier).
- Added `// prettier-ignore` above 7 other pre-existing statements (in
workbook-writer.js, zip-stream.js, xlsx.js) that Prettier's current
version reformats into a shape ESLint's `comma-dangle`/
`space-before-function-paren` rules reject — confirmed via a pristine
origin/master copy that this reformat-then-reject failure is 100%
pre-existing and unrelated to any line this diff touches.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QY55Be9GCxpDat9XQhzeGZ
|
CI is red across every matrix leg, but not from this diff — the workflow uses Local verification (this repo's own test scripts, not CI) is clean on this branch: targeted spec 6/6, Not fixing the workflow itself here (out of scope for XLS-352, and repo-wide — every branch is equally broken). Flagging for whoever owns this repo's CI to bump |
|
Superseded by #3 — same fix, rebased onto |
1 participant
Summary
date) is a per-entry archiver/JSZip option that defaults tonew Date()at append time; a module-levelzip: {date}option is a no-op, so writer output was non-deterministic at DOS-format 2-second granularity (XLS-350's determinism issue).zipEntryDate?: Datewriter option, threaded through both writer paths:stream.xlsx.WorkbookWriter, via a new_append(data, options)choke point every internalzip.appendcall now goes through.Workbook.xlsx.writeBuffer/writeFile, via a newZipWriter._pinEntryDates()called atfinalize()(right beforezip.generateAsync), which sweeps ALLthis.zip.files— including JSZip's auto-created folder entries (xl/,xl/worksheets/, ...) that never pass through.appendand would otherwise keep the wall clock.PinnedWorkbookWritersubclass (packages/shopify-app/app/export-xlsx-file.server.ts), which monkey-patcheszip.appendfrom outside the base constructor because there was no published option to pin entry dates. Once this lands and the server's@protobi/exceljspin is bumped to this commit, that subclass becomes deletable.zipEntryDatechange, confirmed via a pristineorigin/mastercopy of these exact files hitting the same prettier→eslint failure with zero of this diff's changes): one realimport/extensionsviolation, and// prettier-ignoreon 7 pre-existing statements Prettier's current version reformats into a shape ESLint'scomma-dangle/space-before-function-parenrules reject. Out of scope here is the standingfix/prettier-eslint-config-driftbranch, which reformats the whole repo (207 files) — not touched by this PR.Test plan
spec/integration/issues/issue-xls352-zip-entry-mtime-pin.spec.js: both writer paths, 3 arms each (byte-identity across a real >2s delay when pinned, per-entry mtime stamping, positive control proving the option is causally responsible) — 6/6 passing.spec/unit --recursive: 886 passing / 1 pending (pre-existing), no regressions.spec/integration --recursive: 208 passing, no regressions.npm run linton the touched files: clean (0 errors).🤖 Generated with Claude Code
https://claude.ai/code/session_01QY55Be9GCxpDat9XQhzeGZ