Skip to content

feat(push): warn on npm --omit=dev + tsc build script (typescript compilation trap)#1252

Draft
DaveHanns wants to merge 1 commit into
masterfrom
push-warn-omit-dev-tsc-trap
Draft

feat(push): warn on npm --omit=dev + tsc build script (typescript compilation trap)#1252
DaveHanns wants to merge 1 commit into
masterfrom
push-warn-omit-dev-tsc-trap

Conversation

@DaveHanns

Copy link
Copy Markdown
Contributor

Summary

Add a pre-upload sanity check in apify push that warns (never blocks) when
the project's Dockerfile drops devDependencies before running a tsc-based
build. This is one of the most common "worked locally, fails on the platform"
traps for TypeScript Actors, and the platform build error (tsc: not found)
gives no hint that the fix is on the Dockerfile side.

Companion PR to apify/apify-docs #2716 (documents the same trap). Finding
F30-companion from the agentic-actor-development eval sweep.

What changes

  • src/commands/actors/push.ts
    • New exported helper detectOmitDevTscTrap(cwd) — pure function, returns
      a warning message or null. Handles both .actor/Dockerfile and
      ./Dockerfile, folds backslash line-continuations, matches both
      --omit=dev and --production, and refuses to fire on tsc-alias,
      tsdown, or when typescript is duplicated into dependencies.
    • The run() handler calls it right after the "Deploying Actor" log line
      and emits the message through the existing yellow warning() output
      helper (goes to stderr). Push proceeds either way.
  • test/local/commands/push.test.ts
    • Nine unit cases covering positive matches and the four falsy-signal
      guards (typescript in both, no --omit, non-tsc build, missing
      Dockerfile / malformed package.json).

Design notes:

  • Warning only, never a validation error — the platform's Docker build is
    still authoritative, and we don't want a heuristic to block anyone.
  • All file access is guarded with existsSync + try/catch so a missing or
    unreadable Dockerfile / package.json silently no-ops.
  • The tsc regex is a word-boundary-ish match (^|[\s&|;])tsc($|[\s&|;])
    so we don't false-positive on tsc-alias, tsconfig-paths, tsdown, etc.

Test plan

  • oxlint --type-aware clean on push.ts and push.test.ts
  • tsc --noEmit -p . clean
  • Standalone repro of the 9 test cases (positive/negative matrix) all pass
  • CI test suite green (delegating to reviewer / GitHub Actions)
  • Manual: run apify push in a project with a single-stage
    --omit=dev Dockerfile + tsc build script and confirm the yellow
    warning appears before the upload phase
  • Manual: run apify push in a well-configured multi-stage project and
    confirm no warning fires

Cross-references

  • Finding F30-companion (agentic-actor-development eval sweep)
  • Docs companion: apify/apify-docs #2716

…pilation trap)

A common packaging mistake when moving a local TypeScript Actor to the Apify
platform: a single-stage Dockerfile runs `npm install --omit=dev` (or
`--production`), then `npm run build` shells out to `tsc` — but `typescript`
lives in devDependencies, so it was just dropped. The platform-side Docker
build fails with an opaque "tsc: not found" and the user has to guess.

This adds a best-effort local sanity check to `apify push` that inspects the
Dockerfile and package.json before upload and emits a yellow warning (not an
error) when all three conditions hold: (1) `.actor/Dockerfile` or `./Dockerfile`
contains a dev-dep-dropping `npm ci|install|i --omit=dev` (or `--production`),
(2) `scripts.build` shells out to a standalone `tsc` (not tsc-alias, tsdown,
etc.), and (3) `typescript` is in devDependencies but not in dependencies. The
warning points to the two standard fixes: multi-stage Dockerfile with a build
stage that keeps devDeps, or drop `--omit=dev`. Push is never blocked — the
platform build remains the source of truth.

Companion to apify/apify-docs #2716 (docs side of the same finding).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants