fix(build): observe failOnWarn warnings independently of logLevel#960
Draft
spokodev wants to merge 1 commit into
Draft
fix(build): observe failOnWarn warnings independently of logLevel#960spokodev wants to merge 1 commit into
spokodev wants to merge 1 commit into
Conversation
`failOnWarn: true` is documented as a way to fail a build whenever a warning is emitted. In practice it relied on rolldown's defaultHandler to escalate 'warn' into a build error, but when the caller passes `logLevel: 'silent'` rolldown's defaultHandler becomes a no-op. The result is that warnings such as unresolved imports get silently externalised and the build succeeds when the user explicitly opted into failing. Decouple the two by giving rolldown a 'warn' log level whenever failOnWarn is enabled and the user-facing log level would otherwise suppress warnings. This keeps the existing behaviour for callers who do not set failOnWarn and for callers whose logLevel already allows warnings. The escalation path in the onLog handler is unchanged. Add direct-build tests in tests/e2e.test.ts that bypass the testBuild helper (which overrides rolldown's logLevel) so the silent-mode path is actually exercised. The four scenarios cover: silent + failOnWarn + warning (must throw), silent + failOnWarn + clean build (must succeed), silent + failOnWarn=false + warning (must succeed), and warn + failOnWarn + warning (must still throw, regression preservation). Closes rolldown#952
✅ Deploy Preview for tsdown-main ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
tsdown
create-tsdown
@tsdown/css
@tsdown/exe
tsdown-migrate
commit: |
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.
Problem
failOnWarn: trueis documented as a way to fail a build whenever a warning is emitted. In practice it relied on rolldown'sdefaultHandlerto escalate'warn'into a build error, but when the caller passeslogLevel: 'silent'rolldown'sdefaultHandlerbecomes a no-op. So warnings such as unresolved imports get silently externalised and the build succeeds when the user explicitly opted into failing.A common shape where this bites is CI scripts that pass
logLevel: 'silent'to keep their own logs clean andfailOnWarn: trueto fail builds with unresolved imports. The current behaviour means those builds silently pass.Repro
The same call with
logLevel: 'warn'already throws, so the bug only surfaces under silent.Fix
Decouple rolldown's
logLevelfrom tsdown'slogger.levelwheneverfailOnWarnis enabled and the user-facing log level would otherwise suppress warnings:This keeps existing behaviour for callers who do not set
failOnWarnand for callers whoselogLevelalready allows warnings. The escalation path inside theonLoghandler is unchanged.Tests
Four direct-build tests added to
tests/e2e.test.tsthat bypass thetestBuildhelper, which is necessary becausetestBuild'sinputOptionsoverride forces rolldown'slogLevelto'info'and therefore could never reproduce the silent-mode path. The scenarios cover all four axes:failOnWarn+ warning =>build()throws.failOnWarn+ clean build (no warnings) =>build()succeeds.failOnWarn: false+ warning =>build()succeeds (opt-out respected).warn+failOnWarn+ warning =>build()still throws.Without the fix the positive scenario fails on
main(the build resolves silently); the other three pass on baseline already and continue to pass with the patch, confirming the fix narrowly targets the broken path.pnpm lintclean,pnpm typecheckclean, full e2e suite green for all 5failOnWarncases (1 pre-existing + 4 new). The 3 pre-existingexe.test.tsfailures onmainare unrelated to this change.Closes #952
Note on disclosure
Bug surfacing and the repro are from
@tbesedain the issue body. I traced the coupling insrc/features/rolldown.ts, reproduced it via the script above, applied the minimal log-level decoupling, and added the tests. AI assistance was used while drafting parts of the test boilerplate and this PR body; the fix and tests reflect my own engineering decisions.