Exit rather than return in the release decider - #15
Merged
Conversation
`node -e` has no CommonJS module wrapper, so a top-level `return` is a SyntaxError and the decide job died before it chose anything. The tag and dispatch branches both ended in one, which is why the first push to master under the new workflow failed instead of publishing nothing. `process.exit(0)` is what the surrounding code already reaches for on the error paths, and `emit` writes synchronously, so the outputs are on disk before the exit. The mistake was in how this was checked, not only in the code: the script was extracted to a .cjs file and run, and CommonJS wraps a module in a function, which makes a top-level return legal. Verifying it now means running the `run:` block as bash, the way the runner does.
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
1 participant
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.
The first push to master under the new release workflow failed.
decideinrelease-js.ymldied with:node -eevaluates a script without the CommonJS module wrapper, so thetop-level
returnthat ends the tag branch and the dispatch branch is notlegal there. The job never got as far as choosing a package.
The oxc workflow was unaffected, has no such script, and published
solid-layouts-oxc@0.2.3from the same push as intended.The fix
Both branches now end in
process.exit(0), which is the idiom the errorpaths in the same script already use, and which is safe because
emitappends to
GITHUB_OUTPUTsynchronously.How this got through
It was checked by extracting the script to a
.cjsfile and running it.CommonJS wraps a module in a function, so a top-level
returnis legal inthat setting and every path passed. The extraction is what made the test
pass.
Checking it now means running the step's
run:block as bash, the way therunner does. All five paths, against the live registry:
packages=[] publish=false any=false, both packages reported already on npmpublish=truepublish=falseexpected tag solid-layouts-v0.2.1The push path emitting nothing is the intended result: both JavaScript
packages are at 0.2.1 and 0.2.1 is published, so a merge that touches no
version releases nothing.
No version bump: this changes CI only, and no published package contents.