Skip to content

Recheck tar-stream 3.2.x adoption once the upstream typing fixes land #1865

Description

@tada5hi

Tracking issue. #1862 deliberately declined tar-stream 3.2.0 → 3.2.1 from a dependabot group, because adopting it cost five as unknown as assertions in production code for a release with zero runtime change. That decision has an expiry: it should be revisited when the upstream declarations stop disagreeing with their own runtime.

This issue records what to watch, how to recheck, and what the answer should look like — so nobody has to re-derive the analysis.

What to watch

Upstream State at filing Effect on hub
streamx#125 — write()/listeners()/eventNames()/setMaxListeners(), and pipe<S extends Writable> → pipe<S> merged, shipped in streamx 2.28.1 Removes 4 of the 5 assertions. Hub resolves 2.28.0, one patch behind — actionable today.
streamx#126 — Writable#end()'s argument declared required, runtime no-ops on undefined/null open Removes the two entry.end(undefined) workarounds.
tar-stream#181 — Header.type omits 4 values and null; linkname cannot be null; byteOffset missing open Removes the Header narrowing that made the bundled types worse than @types/tar-stream on those fields.

Both open PRs were filed from this work.

What will still need a cast, and why that is fine

container.putArchive(pack, …) in apps/server-core-worker/src/adapters/docker/container-pack.ts.

This one is not a typing bug and no upstream declaration fix will clear it: streamx genuinely has no isPaused, unpipe or wrap, and NodeJS.ReadableStream — which dockerode demands — declares all three. It is safe only because docker-modem calls exactly .on('error') and .pipe(req) (docker-modem/lib/modem.js:369-373).

Note this is the same unsoundness that already sits uncast and uncommented at master-image-builder/handlers/execute/module.ts:129 — tracked separately in #1863. Whoever picks this up should handle both together, or neither.

How to recheck

# 1. Are the upstream fixes released?
npm view streamx versions --json | tail -5
npm view tar-stream versions --json | tail -5

# 2. On a scratch branch: take the bump, and pull streamx forward
#    (streamx is transitive via tar-stream's ^2.15.0, so npm update reaches it)
npm install tar-stream@latest -w apps/server-core-worker -w apps/server-storage
npm update streamx

# 3. Remove every cast, then see what actually fails
npm run build:types -w apps/server-storage
npm run build:types -w apps/server-core-worker

The five call sites, for reference:

  • apps/server-storage/src/adapters/http/controllers/bucket/stream.ts — pipeline(source, entry)
  • apps/server-storage/test/unit/bucket-file.spec.ts — data.pipe(extract)
  • apps/server-core-worker/src/adapters/docker/container-pack.ts — readable.pipe(extract) and container.putArchive(pack, …)
  • apps/server-core-worker/src/app/components/analysis-builder/handlers/execute/module.ts — pack.pipe(createGzip())

Also drop @types/tar-stream from both apps' devDependencies at that point — it is fully shadowed by the bundled types once 3.2.1 is in, and stays in the tree transitively via @types/tar-fs. Sync the lockfile with a plain npm install, never --package-lock-only (it strips the ~126 non-host platform optional entries and breaks npm ci on Linux CI while passing locally on macOS).

Expected outcome, already measured

With tar-stream 3.2.1 + streamx 2.28.1, verified against this repo:

Call site streamx 2.28.0 streamx 2.28.1
pipeline(source, entry) cast compiles
readable.pipe(extract) cast compiles
data.pipe(extract) cast compiles
pack.pipe(createGzip()) cast compiles
container.putArchive(pack, …) cast still fails

Add streamx#126 and the two entry.end(undefined) calls revert to entry.end(). Add tar-stream#181 and Header stops misdescribing its own runtime.

Decision criteria

Adopt when the cost is one assertion (putArchive) against an accurate Header — that is a reasonable trade, unlike the five-against-a-narrowed-Header that #1862 declined. Until then there is nothing to gain: 3.2.x remains types-only, and npm diff should be re-run to confirm that still holds for whatever version is current.

If a future tar-stream release carries real runtime changes, that changes the calculus independently — take it on its merits and solve the typing separately.

Regression net

apps/server-core-worker/test/unit/docker/pack.spec.ts covers the putArchive path end-to-end against a real Docker daemon, and was confirmed to genuinely assert (mutating its expected file size makes it exit 1). apps/server-storage/test/unit/bucket-file.spec.ts covers the tar-extraction path. Both should stay green throughout.

Related, with what each does and does not depend on:

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

dependenciesPull requests that update a dependency file

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions