feat(cli): accept WIREIT_WATCH env var as a --watch escape hatch (#1346)#1418
Open
kiwigitops wants to merge 2 commits into
Open
feat(cli): accept WIREIT_WATCH env var as a --watch escape hatch (#1346)#1418kiwigitops wants to merge 2 commits into
kiwigitops wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the short-term proposal in #1346.
Problem
npm/cli#8071 lands a warning today and will turn into a hard error in npm 12:
npm run build --watchis rejected because--watchis not a recognized npm argument. Until now, wireit relied on npm's old behaviour of silently turningnpm run build --watchintonpm_config_watch=in the script's environment. After npm 12 there is no way to enable wireit watch mode fromnpm run— neither--watch(rejected) nor-- --watch(forwarded to the underlying script, not wireit).@aomarks proposed in #1346:
This PR implements proposal 1.
Change
Two
getArgvOptionscall sites now recognizeWIREIT_WATCH(any non-undefined value) as an additional trigger for watch mode, OR'd with the existing detection:case 'npm': { return { watch: - process.env['npm_config_watch'] !== undefined + process.env['npm_config_watch'] !== undefined || + process.env['WIREIT_WATCH'] !== undefined ? readWatchConfigFromEnv() : false, extraArgs: process.argv.slice(2), }; }And the
parseRemainingArgshelper (used for yarn / yarnBerry / pnpm / nodeRun) gets a matching post-loop check so the escape hatch works on every agent, not just npm:The existing
WIREIT_WATCH_STRATEGY/WIREIT_WATCH_POLL_MSenv vars still control the watch strategy via the unchangedreadWatchConfigFromEnv().Test plan
${agent} WIREIT_WATCH=1 runtest case alongside the existingWIREIT_LOGGERenv-var tests. Runs in the per-agent loop, so it covers npm, yarnClassic, yarnBerry, pnpm, andnode --runin one go.--watchandWIREIT_WATCH_STRATEGYare untouched and should continue to pass — the change is purely additive (an extra OR'd condition).Notes for reviewers
--wireit-watcharg) and 3 (declarative{"watch": true}) are bigger surface-area changes and can land on their own.WIREIT_WATCH=(empty string) also triggers watch mode, because the existing detection uses!== undefinedrather than a truthy check — kept consistent so behaviour is predictable; matches howWIREIT_DEBUG_LOGGERis detected inlogger.ts(viaBoolean(...)) is intentionally NOT used here so an emptyWIREIT_WATCH=set in CI's env still triggers, mirroringnpm_config_watch=.