Skip to content

Fail vite build on rolldown warnings via NODE_ENV=test#37270

Merged
silverwind merged 12 commits intogo-gitea:mainfrom
silverwind:es2022
Apr 21, 2026
Merged

Fail vite build on rolldown warnings via NODE_ENV=test#37270
silverwind merged 12 commits intogo-gitea:mainfrom
silverwind:es2022

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 18, 2026

Fail the vite build on any rolldown warnings when NODE_ENV=test is set. This gate is set on the CI make frontend steps (compliance and e2e workflows) and on the local make test-e2e target, so warnings fail the build both in CI and when running e2e tests locally. Regular make frontend / production builds are unaffected.

Example output:

[plugin test-warning-injector] first synthetic warning
[plugin test-warning-injector] second synthetic warning
transforming...✗ Build failed in 14ms
error during build:
Build failed with 1 error:

[plugin fail-on-warnings]
Error: 2 warnings present
    at PluginContextImpl.buildEnd (vite.config.ts:50:13)
    ...

This PR was written with the help of Claude Opus 4.7

Top-level await (used in frontend-openapi-swagger.ts for the CSS
import) requires es2022. This silences the TOLERATED_TRANSFORM
warning emitted on each build.

Browser support for es2022 is available in Chrome 94+, Firefox 93+
and Safari 15+.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 18, 2026
@silverwind silverwind changed the title Bump Vite build target to es2022 Bump vite build target to es2022 Apr 18, 2026
Install an onwarn handler in commonRolldownOptions that throws when
env.CI is set. In local builds the default handler still prints the
warning, but CI promotes it to a build failure so tolerated warnings
can't silently land on main.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind changed the title Bump vite build target to es2022 Bump Vite build target to es2022 and fail CI on rolldown warnings Apr 18, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 18, 2026

@wolfbeast fyi, this will be for v1.27. Outdated, ignore. We wanted to bump to es2022, but found a way to avoid it.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 18, 2026

Actually, I am not sure whether it really needs es2022

This await import("../../../css/swagger.css"); is only for loading CSS files, and the JS is executed in type=module. So I don't know whether the "await" is really a problem, or just a wrong warning from Vite


OK, it seems that it becomes standard in es2022, while some browsers supported it ahead.

Maybe it's better to find a way to avoid such top-level await and make the CSS importing work (if possible)

Details image

@wxiaoguang wxiaoguang marked this pull request as draft April 18, 2026 02:53
@wxiaoguang
Copy link
Copy Markdown
Contributor

The top-level await import is also caused by incorrect Vite manifest parser, it makes the CSS importing very messy.

@wxiaoguang
Copy link
Copy Markdown
Contributor

No need to use es2022

-> Fix top-level await #37272

@wxiaoguang wxiaoguang closed this Apr 18, 2026
@silverwind silverwind reopened this Apr 18, 2026
@silverwind
Copy link
Copy Markdown
Member Author

The warn-as-error here is still useful.

Comment thread vite.config.ts Outdated
Signed-off-by: silverwind <me@silverwind.io>
@silverwind silverwind changed the title Bump Vite build target to es2022 and fail CI on rolldown warnings Fail CI on rolldown warnings Apr 18, 2026
@silverwind
Copy link
Copy Markdown
Member Author

PR is now only warn-as-err.

@silverwind silverwind marked this pull request as ready for review April 18, 2026 08:27
@silverwind
Copy link
Copy Markdown
Member Author

This await import("../../../css/swagger.css"); is only for loading CSS files, and the JS is executed in type=module. So I don't know whether the "await" is really a problem, or just a wrong warning from Vite

Depends on what vite internally does when it sees TLA with CSS. I think it's likely that some form of TLA persists in the output code, and if that is the case, rolldown is right to warn about it.

@wxiaoguang
Copy link
Copy Markdown
Contributor

        // FIXME: if it throws error here, the real Vite compilation error will be hidden, and makes the debug very difficult
        // Need to find a correct way to handle errors.
        console.error(`Failed to update manifest for ${sourceFileName}`);

Can this also have a proper fix?

Throwing errors in closeBundle will hide the real compilation errors: you can try to introduce some compilation error (e.g.: syntax error in index.ts), make frontend, then manifest.json won't be generated, then readFileSync throws an error, and you are not able to see the real syntax error.

@silverwind
Copy link
Copy Markdown
Member Author

While that case should probably be improved, it's not related to the change here, which is self-contained.

@silverwind
Copy link
Copy Markdown
Member Author

The approach here has one issue, it will only log the first warning and exit. I think a real warn-as-err can only be implemented in vite itself.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 18, 2026

While that case should probably be improved, it's not related to the change here, which is self-contained.

Yes, it's not related to the "rolldown warning", but it is still related to the CI build.

For example: if the manifest.json generation goes wrong, the try-cache in closeBundle will cause more serious problems: it just ignores the exception and reports it succeeds.

I think they have the same point: vite build should report warnings/errors to stop the CI build.

@silverwind
Copy link
Copy Markdown
Member Author

I opened vitejs/vite#22263, I think a proper implementation of this can only come from vite.

@wxiaoguang

This comment was marked as resolved.

@wolfbeast
Copy link
Copy Markdown

@wolfbeast fyi, this will be for v1.27. Outdated, ignore. We wanted to bump to es2022, but found a way to avoid it.

Since I was pinged/subscribed here anyway, we've made more progress on ES standards compliance and some of the DOM issues we ran into before, so for the record, UXP now supports:

  • top-level await in modules
  • everything from ES2016 through ES2023 with the singular exception that finalizationRegistry (part of the strongly discouraged ES2021 WeakRef proposal) isn't supported.
  • ES2024 object.groupBy()
  • Form SubmitEvent API. You should be able to remove the workaround that was put in place for us in Gitea.
  • url.CanParse()

Maybe a casual read-through of our release notes can be useful for the dev team here, to know what's changed on our end.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 18, 2026

we've made more progress on ES standards compliance

Thanks for the detailed info. With that confirmation, we could move to es2023 but as long as no new feature needs it, we ought to keep it at es2020. I will make a PR to remove the SubmitEvent polyfill for v1.27.

Maybe a casual read-through of our release notes can be useful for the dev team here, to know what's changed on our end.

Yes, that changelog is my primary resource for UXP compat issues. I still hope mdn/browser-compat-data#17690 will eventually be fixed.

silverwind added a commit to silverwind/gitea that referenced this pull request Apr 18, 2026
PaleMoon/UXP now supports the native SubmitEvent API (per go-gitea#37270
comment thread), so the polyfill added for it is dead weight. Remove the
`submitEventSubmitter` wrapper, the `initSubmitEventPolyfill` init function,
its two document.body listeners, and the `form._submitter` expando. Call sites
use `e.submitter` directly. `repo-diff.ts` switches from `nodeName` string
comparisons to `instanceof HTMLButtonElement` / `HTMLInputElement` for cleaner
type narrowing.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a CI-only Rolldown plugin to make Vite builds fail when Rolldown warnings are emitted, preventing warnings from being silently ignored in CI.

Changes:

  • Introduces a failOnWarningsPlugin() Rolldown plugin that counts warning-level logs and fails the build at buildEnd.
  • Wires the plugin into commonRolldownOptions only when env.CI is set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread vite.config.ts Outdated
Comment thread vite.config.ts Outdated
Addresses review feedback: calling process.exit(1) from a plugin hook
bypasses Vite/Rolldown cleanup and can truncate logs. Throwing routes
failure through the normal error path.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Comment thread vite.config.ts Outdated
Signed-off-by: silverwind <me@silverwind.io>
@lunny lunny added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Apr 19, 2026
@silverwind silverwind requested a review from bircni April 21, 2026 17:13
@silverwind
Copy link
Copy Markdown
Member Author

One more idea came to mind: fail it when in dev or test instead of only on CI. Not sure if it's possible, but likely.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 21, 2026
Switch from env.CI to env.NODE_ENV === 'test' and set NODE_ENV=test in
the CI frontend/e2e build steps and in the test-e2e Makefile target.
Local `make frontend` stays untouched; e2e runs fail on warnings both
in CI and locally.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind changed the title Fail CI on rolldown warnings Fail vite build on rolldown warnings via NODE_ENV=test Apr 21, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 21, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 21, 2026

Now using NODE_ENV=test to gate the plugin, will fail locally during e2e tests and on ci during the frontend build. vite also has a --mode test, but that is already occupied by vitest so I think this is the next-best solution to notice warnings locally during e2e tests.

Reverted, too complicated, and not backporting.

@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 21, 2026
@silverwind silverwind enabled auto-merge (squash) April 21, 2026 18:05
@silverwind silverwind removed the backport/v1.26 This PR should be backported to Gitea 1.26 label Apr 21, 2026
@silverwind silverwind merged commit c489db4 into go-gitea:main Apr 21, 2026
26 checks passed
@silverwind silverwind deleted the es2022 branch April 21, 2026 18:11
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 21, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 21, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 22, 2026
* main: (25 commits)
  Add URL to `Learn more about blocking a user` (go-gitea#37355)
  fix: use TriggerEvent instead of Event in workflow runs API response for scheduled runs (go-gitea#37288)
  Add event.schedule context for schedule actions task (go-gitea#37320)
  Fix typos (go-gitea#37346)
  Fix an issue where changing an organization’s visibility caused problems when users had forked its repositories. (go-gitea#37324)
  Fail vite build on rolldown warnings via NODE_ENV=test (go-gitea#37270)
  Use modern "git update-index --cacheinfo" syntax to support more file names (go-gitea#37338)
  Fix URL related escaping for oauth2 (go-gitea#37334)
  When the requested arch rpm is missing fall back to noarch (go-gitea#37236)
  Fix `relative-time` error and improve global error handler (go-gitea#37241)
  Enhance styling in actions page (go-gitea#37323)
  fix(oauth): Error on auth sources with spaces (go-gitea#37327)
  Fix actions concurrency groups cross-branch leak (go-gitea#37311)
  Fix bug when accessing user badges (go-gitea#37321)
  Fix AppFullLink (go-gitea#37325)
  Update go js dependencies (go-gitea#37312)
  Update GitHub Actions to latest major versions (go-gitea#37313)
  Revert "Add WebKit to e2e test matrix (go-gitea#37298)" (go-gitea#37315)
  Add `form-fetch-action` to some forms, fix "fetch action" resp bug (go-gitea#37305)
  Move heatmap to first-party code (go-gitea#37262)
  ...
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 23, 2026
* origin/main: (32 commits)
  fix: commit status reporting (go-gitea#37372)
  Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356)
  Fix cmd tests by mocking builtin paths (go-gitea#37369)
  chore: upgrade Go version in devcontainer image to 1.26 (go-gitea#37374)
  Fix button layout shift when collapsing file tree in editor (go-gitea#37363)
  Update `Block a user` form (go-gitea#37359)
  Remove IsValidExternalURL/IsAPIURL and use IsValidURL at call sites (go-gitea#37364)
  Add URL to `Learn more about blocking a user` (go-gitea#37355)
  fix: use TriggerEvent instead of Event in workflow runs API response for scheduled runs (go-gitea#37288)
  Add event.schedule context for schedule actions task (go-gitea#37320)
  Fix typos (go-gitea#37346)
  Fix an issue where changing an organization’s visibility caused problems when users had forked its repositories. (go-gitea#37324)
  Fail vite build on rolldown warnings via NODE_ENV=test (go-gitea#37270)
  Use modern "git update-index --cacheinfo" syntax to support more file names (go-gitea#37338)
  Fix URL related escaping for oauth2 (go-gitea#37334)
  When the requested arch rpm is missing fall back to noarch (go-gitea#37236)
  Fix `relative-time` error and improve global error handler (go-gitea#37241)
  Enhance styling in actions page (go-gitea#37323)
  fix(oauth): Error on auth sources with spaces (go-gitea#37327)
  Fix actions concurrency groups cross-branch leak (go-gitea#37311)
  ...

# Conflicts:
#	services/actions/commit_status.go
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 23, 2026
* origin/main: (204 commits)
  fix: commit status reporting (go-gitea#37372)
  Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356)
  Fix cmd tests by mocking builtin paths (go-gitea#37369)
  chore: upgrade Go version in devcontainer image to 1.26 (go-gitea#37374)
  Fix button layout shift when collapsing file tree in editor (go-gitea#37363)
  Update `Block a user` form (go-gitea#37359)
  Remove IsValidExternalURL/IsAPIURL and use IsValidURL at call sites (go-gitea#37364)
  Add URL to `Learn more about blocking a user` (go-gitea#37355)
  fix: use TriggerEvent instead of Event in workflow runs API response for scheduled runs (go-gitea#37288)
  Add event.schedule context for schedule actions task (go-gitea#37320)
  Fix typos (go-gitea#37346)
  Fix an issue where changing an organization’s visibility caused problems when users had forked its repositories. (go-gitea#37324)
  Fail vite build on rolldown warnings via NODE_ENV=test (go-gitea#37270)
  Use modern "git update-index --cacheinfo" syntax to support more file names (go-gitea#37338)
  Fix URL related escaping for oauth2 (go-gitea#37334)
  When the requested arch rpm is missing fall back to noarch (go-gitea#37236)
  Fix `relative-time` error and improve global error handler (go-gitea#37241)
  Enhance styling in actions page (go-gitea#37323)
  fix(oauth): Error on auth sources with spaces (go-gitea#37327)
  Fix actions concurrency groups cross-branch leak (go-gitea#37311)
  ...

# Conflicts:
#	web_src/js/index-domready.ts
#	web_src/js/markup/content.ts
#	web_src/js/markup/refissue.ts
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 23, 2026
* origin/main: (204 commits)
  fix: commit status reporting (go-gitea#37372)
  Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356)
  Fix cmd tests by mocking builtin paths (go-gitea#37369)
  chore: upgrade Go version in devcontainer image to 1.26 (go-gitea#37374)
  Fix button layout shift when collapsing file tree in editor (go-gitea#37363)
  Update `Block a user` form (go-gitea#37359)
  Remove IsValidExternalURL/IsAPIURL and use IsValidURL at call sites (go-gitea#37364)
  Add URL to `Learn more about blocking a user` (go-gitea#37355)
  fix: use TriggerEvent instead of Event in workflow runs API response for scheduled runs (go-gitea#37288)
  Add event.schedule context for schedule actions task (go-gitea#37320)
  Fix typos (go-gitea#37346)
  Fix an issue where changing an organization’s visibility caused problems when users had forked its repositories. (go-gitea#37324)
  Fail vite build on rolldown warnings via NODE_ENV=test (go-gitea#37270)
  Use modern "git update-index --cacheinfo" syntax to support more file names (go-gitea#37338)
  Fix URL related escaping for oauth2 (go-gitea#37334)
  When the requested arch rpm is missing fall back to noarch (go-gitea#37236)
  Fix `relative-time` error and improve global error handler (go-gitea#37241)
  Enhance styling in actions page (go-gitea#37323)
  fix(oauth): Error on auth sources with spaces (go-gitea#37327)
  Fix actions concurrency groups cross-branch leak (go-gitea#37311)
  ...

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

# Conflicts:
#	web_src/js/index-domready.ts
#	web_src/js/markup/content.ts
#	web_src/js/markup/refissue.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants