Skip to content

make build-theme never cleans, so artifacts from a previous build can leak into the bundle #233

Description

@mmcky

The build-theme target removes only .deploy/<theme> before building. It then runs npm run prod:build, and neither the target nor that script runs npm run clean (rimraf public/build build api), so whatever build/ and public/build already contain survives the build — and Makefile lines 21-22 copy both directories wholesale into the bundle.

Step What it clears
rm -rf .deploy/$(THEME) the deploy directory only
npm run prod:build nothing — prod:copy, build:thebe, build:css, remix build, relative-css-asset-urls.mjs
cp -r public / cp -r build copies both into the bundle, stale files included

The consequence that matters is for A/B comparisons: building one revision, then another, and diffing the rendered output. This was hit while working on #225, where a bundle intended as the "baseline" rendered as the candidate instead, and npm run clean before building fixed it. I have verified the missing clean step and the wholesale copy above; I have not reproduced the exact file that leaked, so I would not want the issue to assert a precise mechanism beyond that. A plain single build is generally fine in practice, since remix build rewrites its own entry bundle each time — the hazard is stale other files and anything that compares two revisions.

That pattern is not exotic here: tests/visual/README.md and playwright.config.ts both document diffing one theme build against another as the way to validate a visual change, and THEME_TEMPLATE exists precisely to support it. So the trap sits directly in a documented workflow, and it fails silently — the comparison produces a plausible-looking result rather than an error.

Suggested fix

Add the clean to the target, so the documented workflow is safe by default:

build-theme: check
	npm run clean
	rm -rf .deploy/$(THEME)

Failing that, a note in tests/visual/README.md's "To diff one theme build against another" recipe and in CONTRIBUTING.md would at least make it discoverable. The target is already slow enough that a rimraf of three directories is not a meaningful cost.

🤖 Generated with Claude Code

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    maintenanceHousekeeping: refactors, tooling, infra, style, env upkeep

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions