Skip to content

docs: correct VARLOCK_ENV test-detection caveat - #1088

Merged
theoephraim merged 4 commits into
mainfrom
fix-varlock-env-test-detection-docs
Sep 15, 2026
Merged

theoephraim merged 4 commits into
mainfrom
fix-varlock-env-test-detection-docs

Conversation

@theoephraim

@theoephraim theoephraim commented Sep 14, 2026

Copy link
Copy Markdown
Member

The VARLOCK_ENV test-detection caveat told users that test runners set NODE_ENV=test after varlock has already loaded and resolved, so detection "may not work in all setups". That is not accurate, and it pushes people toward hand-rolled workarounds (calling execSyncVarlock + initVarlockEnv() themselves) that they do not need.

Both Vitest and Jest set their test signals during their own startup, before config files are loaded. Measured across Vitest 4.1.5 and Jest 30.5.1, NODE_ENV=test is already set in every position where varlock/auto-load is normally imported: the test config file, setup files, and app code under test. Only a node --import / NODE_OPTIONS preload resolves before the runner sets anything.

The caveat is replaced with a table of what each runner actually sets, and where:

Runner Signals available
Vitest, parent and workers VITEST and NODE_ENV=test (workers also get VITEST_POOL_ID)
Jest, worker-scoped code (setupFiles, setupFilesAfterEnv, test files, app code under test) JEST_WORKER_ID and NODE_ENV=test
Jest, parent-scoped code (jest.config.*, globalSetup) NODE_ENV=test only
bun test, AVA NODE_ENV=test only
node --test, Mocha none

Three cases where detection does not fire, none of which were documented:

  1. Runners that set no signal. node --test and Mocha set nothing varlock reads, so VARLOCK_ENV never resolves to test under them.
  2. An inherited NODE_ENV. Runners set NODE_ENV=test only when unset, so a shell or CI value survives and every NODE_ENV only row loses its last signal.
  3. Preloading varlock ahead of the runner. The parent resolves before the runner exists to set anything.

Whenever varlock resolves in a parent process, its values are injected as __VARLOCK_ENV and reused by the workers, so a wrong result propagates even to workers that would have detected test themselves. That propagation is now called out, since it fails silently.

Also promotes a recipe out of the caution box into the @currentEnv section: if you already have your own flag, derive it (APP_ENV=$VARLOCK_ENV) instead of assigning process.env at runtime. Explicit overrides still win, and it avoids the ordering trap where a bundled config (vite/vitest) hoists import 'varlock/auto-load' above any inlined sibling module that was meant to set the flag first. Where your own environment names do not line up with $VARLOCK_ENV, the section now shows remap() as the translation step, which passes unmatched values through unchanged so only the differing ones need listing.

Docs only, no behavior change.

The caveat claimed test runners set NODE_ENV=test after varlock has
already resolved. Measured across vitest 4.1.5 and jest 30.5.1: in every
position where auto-load is normally imported (test config, setup file,
app code under test) NODE_ENV=test is already set.

Rewrite it around the two cases that do fail, and add the derived-flag
recipe for keeping your own env flag name.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
varlock-website 90538ae Commit Preview URL

Branch Preview URL
Sep 15 2026, 08:18 PM

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The revised caveat still classifies Jest globalSetup as a safe setup-file import even though it can run without a test signal.

Reviewed changes Reviewed the updated VARLOCK_ENV guidance and checked its claims against Varlock's builtin detection and injected-graph behavior, plus the documented Jest and Vitest startup contracts.

  • Existing environment flags: Documents deriving APP_ENV from $VARLOCK_ENV while preserving explicit process overrides.
  • Test detection timing: Replaces the general warning with specific preload and inherited-NODE_ENV cases, and explains how an early resolution propagates through __VARLOCK_ENV.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock-website/src/content/docs/reference/builtin-variables.mdx Outdated
Replace the prose caveat with a table of what each runner sets and where.
node --test and Mocha set no signal varlock can read, so test is never
detected under them. Jest's parent-scoped code (config, globalSetup) has
only NODE_ENV, unlike its worker-scoped code.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues; two minor runner-matrix corrections are noted inline.

Reviewed changes Reviewed the documentation updates since the prior Pullfrog review, including the revised runner signal matrix and generalized failure caveats.

  • Added a runner signal matrix: Distinguished Vitest and Jest parent and worker scopes, plus the signals exposed by Bun, AVA, Node's test runner, and Mocha.
  • Clarified detection failures: Applied inherited NODE_ENV behavior across signal-only rows and retained the preload and __VARLOCK_ENV propagation warnings.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock-website/src/content/docs/reference/builtin-variables.mdx Outdated
Comment thread packages/varlock-website/src/content/docs/reference/builtin-variables.mdx Outdated
Vitest workers also get VITEST_POOL_ID. Jest's globalTeardown runs after
the runner has assigned JEST_WORKER_ID=1 on the parent, so it does not
belong in the parent-scoped row alongside config loading and globalSetup.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed the runner-matrix corrections added since the prior Pullfrog review.

  • Corrected Vitest worker signals: Added the worker-only VITEST_POOL_ID signal while retaining the parent and worker VITEST and NODE_ENV=test signals.
  • Narrowed Jest parent scope: Removed globalTeardown from the NODE_ENV=test only row because in-band execution can retain JEST_WORKER_ID through teardown.

Pullfrog  | View workflow run | Using azure/gpt-5.6-sol𝕏

Deriving a flag from $VARLOCK_ENV requires the enum to cover every value
it can produce. remap() passes unmatched values through unchanged, so
only the differing ones need listing.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed the environment-name remapping guidance added since the prior Pullfrog review.

  • Added remapping guidance: Demonstrated translating VARLOCK_ENV=preview to APP_ENV=staging while preserving unmatched values and keeping the resulting enum complete.

Pullfrog  | View workflow run | Using azure/gpt-5.6-sol𝕏

@theoephraim
theoephraim merged commit 1e8d229 into main Sep 15, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant