Skip to content

Merge some standalone Vite entries into index.js#37085

Merged
wxiaoguang merged 33 commits into
go-gitea:mainfrom
silverwind:fixswag
Apr 5, 2026
Merged

Merge some standalone Vite entries into index.js#37085
wxiaoguang merged 33 commits into
go-gitea:mainfrom
silverwind:fixswag

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 2, 2026

Keep swagger and external-render-helper as a standalone entries for external render.

  • Move devtest.ts to modules/ as init functions
  • Make external renders correctly load its helper JS and Gitea's current theme
  • Make external render iframe inherit Gitea's iframe's background color to avoid flicker
  • Add e2e tests for external render and OpenAPI iframe

This PR was written with the help of Claude Opus 4.6

The webpack-to-vite migration broke OpenAPI document rendering in dev
mode because viteDevSourceURL blindly mapped all css/* paths to
/web_src/css/*, but standalone CSS like swagger.css lives in
web_src/css/standalone/ and needs its bundled npm dependencies.

Replace the blind css/ mapping with a generic file-existence check
that works for both CSS and JS. Consolidate JS if-chains into a switch
and add a path traversal guard.

For theme support, load the user's Gitea theme CSS on the /api/swagger
page so swagger.ts can read --is-dark-theme instead of relying on
prefers-color-scheme (which ignores the user's Gitea theme preference).

For the iframe case, inject --is-dark-theme and --color-box-body from
the parent page into the iframe document on load. Hide the iframe with
is-loading until content is ready to prevent color flashes.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 2, 2026
@silverwind silverwind requested a review from Copilot April 2, 2026 18:45

This comment was marked as outdated.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Why not simply merge standalone scripts into the one index.js?

@silverwind
Copy link
Copy Markdown
Member Author

I'm not sure that is actual "simpler".

@wxiaoguang
Copy link
Copy Markdown
Contributor

I'm not sure that is actual "simpler".

You will only have one index.js entry, no need to play the tricks with the files in web_src

@silverwind
Copy link
Copy Markdown
Member Author

  1. You mean merge into IIFE or async index.js?
  2. Will add an e2e test here as well as it seems valuable.

@wxiaoguang
Copy link
Copy Markdown
Contributor

You mean merge into IIFE or async index.js?

Nope. Although I suggested to merge them, you corrected me that it won't work due to index.js must be loaded in <script type=module while IIFE must be loaded before the html body.


I mean merage "devtest" and "swagger" into "index.ts", then they can share the modules, no more vite configs, and maybe fewer chunks.

Since every module in index is chunked, so nothing worse?

@silverwind
Copy link
Copy Markdown
Member Author

I mean merage "devtest" and "swagger" into "index.ts"

index.ts is async but the other standalone endpoints are also async already, so likely can be done without any loading state regressions. Only issue is we run some useless JS that does not apply to the standalone pages, but probably neglible.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 3, 2026

Only issue is we run some useless JS that does not apply to the standalone pages, but probably neglible.

They won't run since these JS won't be loaded on the devtest or swagger page, we can make them lazy-load?

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 3, 2026

I'm talking about all the other init* functions in index.js which will run unavoidably. Swagger/devtest would need a similar init* followed by a lazy-load, introducing 1 extra round trip to the server which is currently avoided by the entry points.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 3, 2026

The only real benefit is deduplication of the big swagger-ui chunk which could be lazy-loaded in two places instead of currently one, saving probably ~200kB output size.

I guess I need to experiment if the extra round-trip is noticable in practice or not.

@wxiaoguang
Copy link
Copy Markdown
Contributor

I'm talking about all the other init* functions in index.js which will run unavoidably

I have added the benchmark, they won't take much time. Most of them do nothing if there is no related element on the page.

And we can migrate them to our data-global-init framework, then maybe can be faster.

Swagger/devtest would need a similar init* followed by a lazy-load, introducing 1 extra round trip to the server which is currently avoided by the entry points.

You can benchmark, less than 0.0...01s

@wxiaoguang
Copy link
Copy Markdown
Contributor

introducing 1 extra round trip to the server which is currently avoided by the entry points.

I don't think anyone can really feel the " 1 extra round trip " on the devtest / swagger page, and I don't think anyone would be affected.

@silverwind
Copy link
Copy Markdown
Member Author

If your latency to the server is 300ms+, it could be noticable. But generally most users will have less latency than that.

@wxiaoguang
Copy link
Copy Markdown
Contributor

If your latency to the server is 300ms+, it could be noticable. But generally most users will have less latency than that.

  1. No user will be affected by the 300ms+ latency on devtest page
  2. For the swagger page: if the latency is 300ms+, then the swagger json is already large enough, would they really care the loading time becomes slightly longer?

@wxiaoguang
Copy link
Copy Markdown
Contributor

2. For the swagger page: if the latency is 300ms+, then the swagger json is already large enough, would they really care the loading time becomes slightly longer?

In this case, the more chunks in "index.js" (caused by the standalone files) will affect users more, for >99% time they are visiting the main site's pages, but not swagger page.

So even the network latency is high, the merged solution still brings best user experience.

… resize during loading

- Add matchMedia change listener so swagger dark-mode class updates when
  system color scheme changes at runtime (relevant for auto theme)
- Remove is-loading guard on iframe resize messages so the correct height
  is set before the iframe becomes visible

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 3, 2026

It seems we have to load iife (needed for jQuery, which is needed by fomantic import in index.js), so it will be loading considerably more js on those pages, but I guess it wont matter. window.config also may need to be stubbed.

@wxiaoguang
Copy link
Copy Markdown
Contributor

window.config also may need to be stubbed.

head_script.tmpl already includes iifs.js and window.config, just simply include it.

Remove standalone JS entry points (swagger, devtest, external-render-iframe)
from the Vite config. Move their init logic into modules/ and register them
in index.ts's callInitFunctions array. Standalone pages (openapi iframe,
external render iframe) now load iife.js + index.js via head_script template.

- Move swagger/devtest/external-render-iframe JS into modules/
- Delete web_src/js/standalone/ and web_src/css/standalone/
- Keep swagger.css and devtest.css as CSS-only Vite entries
- Swagger template and openapi.go use head_script + index.js
- Pass pre-rendered HeadScriptHTML from router to markup renderer
- Use `isDarkTheme()` with matchMedia fallback for swagger dark mode
- Use `data-gitea-theme-dark` attribute for swagger CSS overrides
- Use CSS vars instead of hardcoded colors in swagger.css
- Guard customElements.define to prevent re-registration in iframes
- Suppress context-canceled proxy errors in vitedev.go

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind changed the title Fix Swagger/OpenAPI rendering in dev mode and improve theme support Fix Swagger/OpenAPI rendering and merge standalone Vite entry points Apr 3, 2026
Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

Standalone entry points removed. CSS files are kept as entry points to avoid FOUC, but could also make it load index.css if you prefer, but it'd be 99% dead CSS.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 5, 2026

Move standalone/{swagger,devtest,external-render-iframe}.ts to modules/ as init functions

During the refactoring, I found that keeping swagger as standalone still seems to be a better choice for now. At the moment, it doesn't share anything with "index.ts" and its modules, and the swagger ui page doesn't need any code from main site other than "js-yaml".

In the future, if the swagger ui page needs to use main site's modules, we can migrate it at that time (and the newly added HINT comment can help people to understand the problem).

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 5, 2026

Found another problem: inconsistent output directories for JS

Old:

  • js/index.ts
    • output: js/index.js
    • imported by /assets/js/index.js
  • js/features/eventsource.sharedworker.ts
    • output: js/eventsource.sharedworker.js
    • imported by /assets/js/eventsource.sharedworker.js
  • js/standalone/external-render-iframe.ts (when using iife)
    • output: js/standalone/external-render-iframe.js
    • imported by /assets/js/external-render-iframe.js

To make everything clear and consistent, now, all "entry" files are moved to the web_src/js directory, no sub-directory anymore.


If any one would like to add sub-directories in the future, all the paths must be kept consistent: if an entry is in a sub-directory, its output should also be in the same sub-directory, and the frontend should also use the sub-directory to import it.

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

Copilot reviewed 32 out of 34 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

web_src/js/external-render-helper.ts:23

  • data-gitea-theme-dark is only set when the query param indicates dark mode. When it’s false, the attribute is left unset, which breaks CSS selectors that rely on explicit "true"/"false" (e.g. GitHub-style dark/light image switching based on html[data-gitea-theme-dark="false"]). Consider always setting the attribute to "true" or "false" based on the param value.
    web_src/js/external-render-helper.ts:32
  • themeUri is taken directly from a query parameter and injected into a . A crafted /render/ URL could point this to an arbitrary external CSS URL, causing the sandboxed iframe to make cross-origin requests (privacy leak via Referer) and allowing untrusted CSS to be applied. Consider validating themeUri before using it (e.g. only allow same-origin /assets/css/theme-.css (and/or /web_src/css/themes/ in dev), otherwise ignore).

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

Comment thread modules/markup/render.go Outdated
Comment thread vite.config.ts
Comment thread modules/markup/external/openapi.go
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 5, 2026

I have tested:

  1. make watch-frontend
  2. make frontend

X

a. embedded swagger
b. gitea api swagger
c. other external render

X

A. light theme
B. dark theme


Will merge if tests pass, to catch 1.26 release.

@wxiaoguang wxiaoguang changed the title Merge standalone Vite entry points into index.js Merge some standalone Vite entries into index.js Apr 5, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 5, 2026 18:32
@wxiaoguang wxiaoguang added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 5, 2026
@wxiaoguang wxiaoguang merged commit a893811 into go-gitea:main Apr 5, 2026
26 checks passed
@wxiaoguang wxiaoguang deleted the fixswag branch April 5, 2026 19:13
@GiteaBot GiteaBot added this to the 1.26.0 milestone Apr 5, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 5, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 7, 2026
* main:
  Repair duration display for bad stopped timestamps (go-gitea#37121)
  Add terraform state registry (go-gitea#36710)
  Add placeholder content for empty content page (go-gitea#37114)
  Improve control char rendering and escape button styling (go-gitea#37094)
  Add gpg signing for merge rebase and update by rebase (go-gitea#36701)
  Move package settings to package instead of being tied to version (go-gitea#37026)
  Merge some standalone Vite entries into index.js (go-gitea#37085)
  Update Nix flake (go-gitea#37110)
  [skip ci] Updated translations via Crowdin
  Fix the wrong push commits in the pull request when force push (go-gitea#36914)
@silverwind
Copy link
Copy Markdown
Member Author

Why is CORS being enabled in the Vite server? Surely we can do without CORS?

@wxiaoguang
Copy link
Copy Markdown
Contributor

Why is CORS being enabled in the Vite server? Surely we can do without CORS?

Surely you can't.

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/code-linting type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants