Release/2.0.3 - #1630
Merged
Merged
Release/2.0.3#1630
Conversation
…ated components (#1530) Fix #1524 Root cause DeepSeek's reasoning models stream reasoning_content alongside content. Answer ignored it, so follow-up requests failed with 400: The reasoning_content in the thinking mode must be passed back to the API, and the thinking text was never shown or saved. Fix - Capture reasoning_content from the stream and pass it back to theAPI on subsequent rounds. - Persist it with the conversation (new DB column via migrationv2.0.2). - Render it in the chat UI as a collapsible "Thinking…/Thoughts"panel above the answer. Compatibility Nullable column, omitempty field, UI hides the panel when empty — old conversations and non-reasoning models behave exactly as before. Demo https://github.com/user-attachments/assets/49b1a2a1-9133-4ac2-bbeb-860215a50285
…ansliteration (#1526) # fix: avoid `topic` fallback for non-Latin titles via pragmatic ASCII transliteration > **Scope update (in response to review):** this PR is intentionally broader than its original "Arabic-only" framing. The implementation changes URL slug generation for **every non-Latin, non-CJK script** that `slugify` previously stripped — see *Scope* below for the explicit list. The goal is *not* linguistically correct romanization; it is "avoid collapsing to `/topic` by producing a usable ASCII slug." ## What this PR is (and isn't) **Goal:** when a question title contains characters outside Basic Latin / Latin Extended / CJK Han, generate a URL slug that is a deterministic ASCII approximation instead of letting `slugify` strip everything and falling back to the literal `"topic"`. **Non-goal:** this is *not* a linguistically correct multi-language romanizer. The output is a machine-acceptable ASCII slug, not what a native speaker would choose. For example, `こんにちは` → `konnichiha` (not the more natural `kon'nichiwa`), `ไทย` → `aithy` (not `thai`). Treat the slug as an opaque, stable, indexable identifier — the path-after-`/questions/<id>/` is for SEO and shareability, the canonical reference is always the ID. ## The bug Pure non-Latin titles previously got stripped by `slugify.Slugify`, hit the empty-result fallback in `htmltext.UrlTitle`, and collapsed to the literal slug `"topic"`. On a live multilingual site, every Arabic / Thai / Japanese-hiragana / Korean / Hebrew / Cyrillic question ended up at `/questions/<id>/topic`. ## The fix `UrlTitle()` gets a `convertNonLatin` pre-step that mirrors the existing `convertChinese` pre-step pattern, using `github.com/mozillazg/go-unidecode` (same author as `go-pinyin` already in the repo, to minimise new-dep friction). ``` UrlTitle(title) → convertChinese(title) // pre-existing: Han-block → pinyin → convertNonLatin(title) // NEW: detect non-Latin letters → unidecode to ASCII → clearEmoji / slugify / url.QueryEscape / cutLongTitle (unchanged) ``` The non-Latin detector skips ASCII, Latin-1 Supplement, Latin Extended-A/B, and CJK Han. Inputs that hit none of those non-Latin letter categories short-circuit and return unchanged, so Latin-only and Chinese-only inputs remain byte-identical (pinned by tests). ## Scope — what scripts are affected This PR changes behavior for **any** title containing letters in scripts that `slugify` doesn't handle. Confirmed by tests in `pkg/htmltext/htmltext_test.go`: | Script | Example title | Before | After | | --- | --- | --- | --- | | Arabic | `كيف حالك` | `topic` | `kyf-hlk` | | Mixed Latin + Arabic | `مرحبا hello` | `hello` | `mrhb-hello` | | Thai | `ไทย ไทย` | `topic` | `aithy-aithy` | | Japanese hiragana | `こんにちは` | `topic` | `konnichiha` | | Korean | `안녕하세요` | `topic` | `annyeonghaseyo` | | Hebrew | `שלום עולם` | `topic` | `shlvm-vlm` | | Cyrillic | `Привет мир` | `topic` | `privet-mir` | **Unchanged:** | Case | Behavior | | --- | --- | | Pure Latin (`hello world`) | unchanged → `hello-world` | | Pure Chinese (`这是一个，标题，title`) | unchanged → `zhe-shi-yi-ge-biao-ti` (pinyin path) | | Japanese with Han-block kanji (`日本`) | unchanged → `ri-ben` (caught by pre-existing pinyin path; treated as Chinese reading, not Japanese — a pre-existing limitation, **not** introduced by this PR) | | Emoji only (`😂😂😂`) | unchanged → `topic` | | Empty / whitespace | unchanged → `topic` | ## Transliteration quality — explicit acknowledgement `go-unidecode` is a generic Unicode → ASCII approximation. It is **not** a per-language romanization library. Specifically: - It will pick *one* approximation per codepoint regardless of language context. `ใ` → `ai` (Thai romanization is `i` or `ai` depending on standard), `한` → `han`, `語` → `Yu` (Chinese pinyin reading even when used in Japanese), etc. - The result is *good enough* to be a stable, URL-safe, human-recognizable handle, but speakers of the source language will not consider it "correct." - It is deterministic, so the same title always produces the same slug — important since `url_title` is recomputed on every request. If maintainers prefer to scope this PR more narrowly (e.g. Arabic only, and reject Thai/Hebrew/Cyrillic/etc.), the detector in `containsNonLatin` can be tightened to specific Unicode blocks — but that means the other scripts continue to collapse to `topic`, which is the bug we're trying to fix. I'd argue the broader fix is preferable to a piecemeal one, but happy to narrow if you want. ## Live deployment / real-world verification This patch has been running in production on **[ask.namasoft.com](https://ask.namasoft.com)** (an Apache Answer instance we operate) since deployment, built directly from this branch via `docker compose build`. The site hosts Arabic-language questions, so the fix exercises the affected code path on every page load. Sample question URL on the deployed instance: > `https://ask.namasoft.com/questions/10010000000000115` The slug in the URL is the transliterated Arabic title rather than `topic`. No data migration was needed since `url_title` is computed on every request from `Title` and never persisted (see *Why this is safe to ship* below). ## Admin-configurable The transliteration is gated by a package-level `atomic.Bool` (default **on**, since the current behavior is objectively broken for affected users): - `htmltext.SetTransliterateNonLatin(enabled bool)` - `htmltext.IsTransliterateNonLatinEnabled() bool` This is deliberately the minimum surface needed to satisfy "the setting must be readable from `UrlTitle()`". A follow-up PR can add an admin UI section that calls `SetTransliterateNonLatin` on save and on startup, without having to re-plumb every `htmltext.UrlTitle` call site through `context.Context`. **Default choice — please confirm:** I picked **default-on** because the existing `topic` behavior is a bug for affected users. If you'd prefer default-off for strict backward compat on existing installs, flip the `init()` in `pkg/htmltext/htmltext.go` to `Store(false)` and surface the toggle as opt-in. ## Why this is safe to ship - `url_title` is **not** a persisted column. It's not on the `Question` entity in `internal/entity/question_entity.go`, no migration has ever added/dropped it, and every call site (`question_service.go`, `revision_service.go`, `vote_service.go`, search/report/review/rank/comment services, controllers, repos) recomputes it from `Title` at response-build time via `htmltext.UrlTitle(...)`. - That means the fix is read-only: existing rows light up with correct slugs on the next request, with no migration and no data rewrite. - Rollback is just redeploying the prior image; nothing on disk changes. ## Test coverage `pkg/htmltext/htmltext_test.go`: - **`TestUrlTitleTable`** — table-driven, one case per affected script (the full matrix above), plus: - `empty` → `topic` - `pure latin unchanged` → byte-identical to pre-fix - `pure chinese unchanged` → byte-identical to pre-fix (pins existing pinyin behavior) - `japanese kanji goes through pinyin path unchanged` → documents the pre-existing Han-block limitation - `emoji only falls back to topic` → unchanged - `long arabic truncates at cutLongTitle boundary` → exercises the 150-byte cap and UTF-8 boundary safety - **`TestUrlTitleTransliterationToggle`** — with the toggle off, non-Latin titles collapse to `topic` (pre-fix behavior); with it on, they transliterate. - Existing `TestUrlTitle` left untouched. Test plan for reviewers: - [ ] `go test ./pkg/htmltext/...` — all pass - [ ] Visit the live sample URL above and confirm slug is transliterated, not `topic` - [ ] Verify Chinese / Latin / emoji-only / empty behavior is byte-identical to `main` (covered by table tests) ## Out of scope (intentionally) - No admin UI / site setting plumbing in this PR — see *Admin-configurable* above. Happy to do the React `Non-Latin Languages Handling` admin page + `SiteType` + service / controller / migration in a follow-up if maintainers want it. - No change to the `"topic"` empty-result fallback. - No plugin interface for slug generation — mirrored the existing `convertChinese` pre-step pattern instead. - No per-language romanization library — this is an explicit non-goal; see *Transliteration quality* above. ## Issues / discussion I didn't find an existing upstream issue covering this — happy to be pointed at one if there is. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: LinkinStars <linkinstar@foxmail.com>
The ownership check added to AcceptAnswer compared the answer's QuestionID against the request's QuestionID directly. When short links are enabled, answerRepo.GetByID re-encodes QuestionID to its short form while the controller de-shorts req.QuestionID to its long form, so the two encodings of the same question never matched and every accept returned "Answer do not found". Normalize both ids via uid.DeShortID before comparing, preserving the privilege-escalation guard for answers that truly belong to another question.
Replace OptionalBool with an explicit require_email_verification value for the login settings save request while keeping legacy read defaults intact. Use positive RequireEmailVerification naming through the registration flow and inline the site setting mapping. Add validation, save-path, and registration coverage for the explicit email verification setting.
…il worker for test
# Conflicts: # docs/release/LICENSE # internal/service/notification/new_question_notification_test.go
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
The search term was formatted into LOWER(%s) and passed as the *value* of the
LIKE, so the function name ended up inside the pattern:
slug_name LIKE '%LOWER(coco)%'
That can never match. Only the display_name clause did any work, and LIKE is
case-sensitive on Postgres, so searching a tag by the name it is written in
returns nothing:
slug_name=Coco -> matches
slug_name=coco -> no match
Tags are lower case by convention, so lower case is what users type, and the
filter appears to report that no such tag exists.
Lower both sides instead. The term normalisation is extracted so it can be
covered by a test without a database.
Add a table-driven test covering the markdown link destination check before replacing the govalidator dependency with stdlib logic. Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
Signed-off-by: LinkinStars <linkinstar@foxmail.com>
Users whose username contains an uppercase letter cannot save their
profile at all — not even when they leave the username untouched,
because the whole form is validated on submit.
The check in profile settings is the only one of four that rejects
uppercase:
| where | pattern | uppercase |
|---|---|---|
| `ui/src/pages/Users/Register/components/SignUpForm/index.tsx` |
`/^[\w.-\s]{2,30}$/` | allowed |
| `ui/src/pages/Install/components/FourthStep/index.tsx` |
`/^[\w.-\s]{2,30}$/` | allowed |
| `pkg/checker/username.go` | `^[\w.\- ]{2,30}$` | allowed |
| `ui/src/pages/Users/Settings/Profile/index.tsx` | `/[^a-z0-9\-._]/` |
**rejected** |
So one can sign up as `MaxMustermann`, and from then on the profile page
is locked. The error message points at the username field without saying
why a name the server itself issued is suddenly invalid.
I ran into this migrating a 26-year-old forum to Answer: 3368 of 5479
accounts carry uppercase letters in names that have been in use for two
decades.
## Proposed Changes
- add the ignore-case flag to the username check in profile settings,
bringing it in line with registration, installation and the server
- permits nothing the server would reject; no data or API behaviour
changes
An alternative would be to reuse the exact pattern from `SignUpForm`,
but that also allows spaces, which felt like a larger change than this
fix needs. Happy to switch if you prefer the patterns to be literally
identical.
Co-authored-by: Besser Sehen Landshut <hallo@bessersehen.la>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n is enabled (#1597) Fixes #1532. ### Problem The `semantic_search` MCP tool was always advertised to the model, even when no VectorSearch plugin was enabled. The model could select it, the call reached `EmbeddingService.SearchSimilar`, and every such conversation logged `semantic search failed: semantic search is not available: no vector search plugin is enabled` — polluting logs and wasting a tool-call round trip on a capability that does not exist in the deployment. ### Fix - `EmbeddingService.Available()` reports whether a VectorSearch plugin is currently enabled. - `MCPController.SemanticSearchAvailable()` exposes that to the AI chat. - `getMCPTools()` omits the `semantic_search` tool when unavailable; all other MCP tools keep working exactly as before. - The default AI prompts (zh/en, and admin-custom prompts) drop the `semantic_search` instruction lines in the same situation, so the model is no longer nudged toward a missing capability. ### Testing - New unit tests: tool advertisement with/without the capability, prompt-line stripping, and prompt adaptation (`internal/controller/ai_tools_test.go`). - `go test ./internal/controller/` — all pass. Co-authored-by: LinkinStars <linkinstar@foxmail.com>
CommentURL called AnswerURL(permalink, siteUrl, questionID, answerID, title), but AnswerURL takes (permalink, siteUrl, questionID, title, answerID), so the last two arguments were reversed for every comment left on an answer. The answer route is /questions/:id/:title/:answerid, so the answer id landed in the title slot and the title landed in the answer id slot. Under the numeric permalink settings the title was run through uid.DeShortID and came out as an unrelated number. Under the short id settings uid.EnShortID returned it unchanged, so the raw title, spaces and all, was written into the path. The broken link reaches the new comment notification email built in internal/service/export/email_service.go and the CommentUrl handed to notification plugins in internal/service/notification_common/notification.go. A comment on a question takes the other branch and was already correct. Pass title and answerID in the order AnswerURL declares them, and add pkg/display/url_test.go covering both branches across all four permalink settings.
Add a new `persistence.existingClaim` value that, when set, causes the deployment to mount the specified PVC instead of the chart-managed one and skips creation of the PVC resource entirely. Signed-off-by: Steven Koo <steven.koo@emerson.com>
The Gravatar specification hashes the trimmed, lowercased address. GetAvatarURL only trimmed it, so an account whose stored address contains an uppercase letter hashed to an address Gravatar does not know: the user's avatar was never found and the identicon fallback was rendered instead. selectedAvatar recomputes this URL from the stored address on every response, so this affected both default_avatar: gravatar and an explicit per-user avatar.type: gravatar, and a user could not work around it by re-selecting Gravatar in their profile. Nothing in the backend normalises a stored address, and the external login path copies the provider's address verbatim, so accounts created through an OIDC/OAuth2 connector inherit whatever casing the identity provider sends. The web UI already lowercases before hashing, so the Settings -> Profile preview showed the user's real avatar while every other surface showed an identicon. This removes that disagreement. The hash is computed on read, so existing accounts resolve correctly as soon as this ships. Stored addresses are left untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two things the server depends on are invisible to the build. Both fail silently, so a green build and a working dev server actively disguise them. The server parses the script and stylesheet paths out of the built index.html and reuses them on every server-rendered page. That parse is coupled to the exact attribute set and attribute order the frontend build writes into those tags. A build that emits a different tag shape still succeeds, the dev server still works, the binary still compiles, and the pages simply render with no scripts and no stylesheet. TestGetStyleResolvesBuiltAssets asserts the parse still finds them. Languages other than the default one are loaded with a template-literal dynamic import through an alias that points outside the frontend root. A bundler that cannot enumerate that pattern still builds and still serves a working app; the resources never arrive, and only for non-default languages, so a smoke test in the default language misses it. check-locale-resolution.js bundles that same import with the project's own configuration, runs it, and requires two languages to resolve to distinct translated content. Both run through make check-ui. check-built-assets.sh --self-check confirms the asset check still fails on tag shapes the parser cannot read, so a check that quietly stopped asserting anything is distinguishable from a passing one. (cherry picked from commit 71cd924, internal/controller/template_controller_test.go only)
GetStyle scraped index.html with regexes matching one exact tag shape: classic scripts with defer first, and stylesheet links with href before rel. Any bundler emitting a different shape returned nothing, and server-rendered pages would load with no JavaScript and no stylesheet while every build step still reported success. The tags are now read from the parsed document, so attribute order, attribute set and quoting no longer matter. header.html emits the scraped paths as script tags itself, and those were classic scripts. A module bundle loaded that way fails on its first import, so fixing only the parsing would have left server-rendered pages broken; the tag is now declared as a module. The self-check fixtures are replaced. The previous two asserted failure on module scripts and on rel-before-href, both of which the parser now accepts, so they would have inverted into false alarms. The replacements cover a stylesheet with no script, a script with no stylesheet, and an inline script with no src. golang.org/x/net moves to a direct requirement, matching its use here. (cherry picked from commit eab6f9d, go.mod and internal/controller/template_controller.go only)
GetStyle returned a single stylesheet path because the previous build emitted exactly one. The current build emits two, so server-rendered pages loaded partially unstyled while every build step still reported success. The stylesheet is now a list, mirroring how script paths are already collected and prefixed, and the template renders one link per entry. This is the same assumption as the tag-shape one fixed earlier: the server encoded a property of one bundler's output, here that there is exactly one entry stylesheet. (cherry picked from commit d66e21b)
The linter configured for this repository flags the slice-building form, so make lint fails on it. (cherry picked from commit 1cfd5da)
Asserting that the parsed stylesheet list is non-empty leaves the exact regression this repository already hit uncovered: a parser that stops at the first stylesheet returns a one element list, satisfies every existing assertion, and silently drops the rest of the page's CSS. Count the declarations again by a cruder method than the parser uses and require the two to agree, so the parser has to be checked against something other than itself. The count is a lower bound: a build that quotes attributes differently drives it to zero and it stops constraining, which is why it supplements the shape-independent assertions rather than replacing them. Verified by reintroducing the truncation and watching this fail. (cherry picked from commit 9232cbd)
The GetStyle comment framed the DOM walk around attribute order, attribute set, and quoting, the same properties the old regex parser depended on, without stating that the new parser ignores all of them. check-built-assets.sh described its self-check as failing when asset tags change shape, but the fixtures test a missing script or stylesheet tag, and the parser accepts any shape as long as the tag is present. Comments now state the actual constraint: tag shape does not affect parsing, and the guarding check fails when a build is present but a required script or stylesheet tag is missing from it. (cherry picked from commit 3bc12e8, internal/controller/template_controller.go only)
Fixes #1580. Part of #1578. Step 2 of 3, atomic. Depends on #1582 (the Go asset contract, now merged into `dev`), which is why the server-side parsing does not appear in this diff. Replaces `react-scripts` and `react-app-rewired` with Vite. The configuration preserves the output contracts the server depends on: the build directory stays at `ui/build`, which `ui/static.go` embeds, and emitted assets stay under `static/`, which `internal/router/ui.go` serves as a route. Files keep the `static/js`, `static/css` and `static/media` grouping, which the `analyze` script matches on; the ignore rules for `ui/build`, previously pinned to that layout's directory depth, now ignore everything under `build` except the tracked favicon. Production sourcemaps stay on, and the `REACT_APP_` prefix is retained so `ui/scripts/env.js` remains the single source of truth for configuration shared with the server. That same script's `public_url` value now also drives Vite's `base` configuration, and the manifest link in `ui/index.html` uses Vite's `%BASE_URL%` macro instead of a hardcoded path, so a non-root deployment keeps every asset and manifest reference prefixed the way it did under the previous toolchain's `PUBLIC_URL` wiring. Agentic tooling did the mechanical work in this series; every change was reviewed by a human before being committed. ### The one server-side line `header.html` re-emitted the paths `GetStyle()` finds as classic scripts. An ES module loaded through a classic script tag fails on its first import, so the tag is now declared as a module and carries `crossorigin`, matching the tag Vite's own build emits for the client-rendered entry point. Rewriting the built `index.html` instead was not an option, because `internal/router/ui.go` serves that same file to boot the SPA and it cannot misdeclare its own script type. A `type=module` script fetches in CORS mode, so any CDN origin serving these files needs to send the matching CORS headers, with or without `crossorigin` present; default same-origin deployments are unaffected. The note for CDN users is in the CDN plugin READMEs, apache/answer-plugins#326, per the placement decided on #1567, and the release notes for the version that ships this should carry a short pointer to it. ### Route code splitting Routes loaded pages with `` lazy(() => import(`@/pages/${pagePath}`)) ``. That shape cannot be statically analyzed, so no page received its own chunk and the specifier reached the browser untransformed, leaving every lazily routed page unable to load. Pages are now enumerated with a bounded glob covering the three directory depths routes actually use, excluding component subtrees so their own index files do not become route chunks. A page path with no matching module now rejects with the requested path and the list of known keys, surfacing through the existing route error boundary. ### Behaviour changes, called out deliberately **The custom stylesheet link now derives its href from the build's own base, not a separately computed value.** `PageTags` built the `/custom.css` link from `process.env.PUBLIC_URL`, which the previous toolchain exposed with its trailing slash already stripped; at the default configuration that value was an empty string, resolving to `/custom.css`. `import.meta.env.BASE_URL`, the direct equivalent under the new toolchain, keeps the trailing slash, so substituting it in the same place would resolve the same default configuration to `//custom.css` instead. The trailing slash is stripped explicitly before the substitution, reproducing the previous output at the default configuration and staying correct away from it. The output here is unchanged; only the mechanism it depends on is. **Two routes were dead ends and now fail loudly instead of silently.** `pages/403` and `pages/Admin/UserOverview` both referenced modules that did not exist in the tree, and both failed the same way under the previous toolchain, silently rendering blank. With the glob above, both now report the missing path through the route error boundary. Step 3 repoints `pages/403` at `pages/404/403`, an existing component, so that route renders; `pages/Admin/UserOverview` stays a visible gap because creating the missing page is a content decision. Neither is a regression. **The markdown editor now renders its themed background.** `src/components/Editor/index.scss` read `var(-bs-body-bg)` with a single leading dash. That is not a valid custom property reference, so the declaration was discarded and the editor never received the background it asks for. The previous CSS minifier accepted the invalid value; the current one rejects it outright and fails the build (`[lightningcss minify] Unexpected token Ident("-bs-body-bg")`), which is how it surfaced and why the one-line fix is part of this PR rather than a follow-up. Fixing it changes rendering. ### Dependency removals `react-scripts`, `react-app-rewired`, `customize-cra` and `config-overrides.js` are gone, and `yaml-loader` is replaced by the equivalent Vite plugin, pinned to the schema the previous loader used so bare dates and merge keys keep parsing the same way they did before. The scaffold test the old toolchain generated (`App.test.tsx`) and its jest packages go with it; this project has no test runner and no unit tests, before or after. Three removed packages were already inert before this migration. Both purgecss packages were declared but wired nowhere: no postcss config exists, the overrides file never referenced them, and no script invoked them. `buffer` was aliased and provided as a global, but no application source uses it, and the one dependency requiring it declares `buffer: false` in its own browser field. `sass` and `@types/node` are raised to the versions the toolchain requires; the previous `sass` predates the async compiler API it now calls. `sass` is pinned below the release that begins deprecating `@import`, which this project uses across 30 files. Migrating those to `@use` is a separate concern. Bootstrap's own Sass internals print dozens of dependency deprecation warnings on every build, unrelated to anything in this project's own styles; `vite.config.mts` sets `css.preprocessorOptions.scss.quietDeps: true` to silence those specifically while still surfacing warnings from this project's own stylesheets. One such app-own warning remains in this PR's build output, a mixed-declarations notice from `Comment/index.scss`; the one-line reorder that clears it is a step 3 nit. The eslint config no longer extends `react-app/jest`, which shipped inside `react-scripts` and configured rules for a test suite this project does not have. ### A failure found only by running the built application With the build green and the dev server working, the built application did not boot. React never mounted, the page showed its loading spinner indefinitely, and the browser console was empty. `i18next` attaches its resource-store methods to the instance inside `init()`. The builtin plugins register their translations while their modules are being evaluated. Whether that happens before or after `init` depends on how the bundler groups and orders chunks, so the previously working order was incidental rather than guaranteed. When it inverts, the registration throws while the entry module is still evaluating, which takes the application down before it mounts and produces no console output. Registration now happens immediately only when there is an initialised instance to register into, and otherwise falls to the `initialized` handler the code already installed, which is correct in either order. The check that guards both orders is part of step 3. ### Parity fixes found in review **Bootstrap icon fonts.** The bootstrap-icons stylesheet points at font files under a path the bundler could not resolve on the first migration pass, so the build silently emitted zero font files and every icon rendered as a missing-glyph box. The stylesheet now overrides the package's font-directory variable to a path Vite can resolve; the fonts are emitted, and the boot test below confirms they are served. **Type checking.** `pnpm build` now also runs `tsc --noEmit` before the bundler runs, and is clean today. The previous toolchain ran its checker in the dev server (blocking) and during builds (downgraded to warnings by this project's `TSC_COMPILE_ON_ERROR` setting); none of that carried over when the bundler changed, so until this fix a type error shipped with no signal at all. **Yaml parsing.** The Vite yaml plugin defaults to js-yaml's more permissive schema, which resolves bare dates to JS `Date` objects and enables merge keys; the previous loader's schema kept both as plain strings. The plugin is now pinned to that same schema. **Declared Node range.** The bundler's declared support range is `^20.19.0 || >=22.12.0`. `ui/package.json`'s own `engines.node` allowed `>=20`, which admits versions below that floor, and now matches it exactly. The release workflow's pinned Node, which also sat below the floor, is raised to `20.19.0` to match. That version bump is the only change this PR makes anywhere under `.github/`; it does not add a job. **Typed environment variables.** `import.meta.env.REACT_APP_*` previously typed as `any`, since no `ImportMetaEnv` augmentation existed; a typo'd key would compile clean and only surface as a missing value at runtime. The two keys the app reads this way, `REACT_APP_API_URL` and `REACT_APP_BASE_URL`, are now declared, with `strictImportMetaEnv` enabled so an undeclared key is a type error instead of a silent `any`. ### Verification On `dev` at `ace02c49`, which includes step 1, plus these commits: `pnpm install --frozen-lockfile` under the pinned `pnpm@9.7.0` and `pnpm build` (which now includes `tsc --noEmit`) complete; the build prints three warnings, each once: `front-matter`, a dependency unrelated to this project's own pinned `js-yaml@^4.1.0`, pulls in a legacy `js-yaml@3.x` copy whose `buffer` import gets externalized for browser compatibility; the Sass mixed-declarations notice from `Comment/index.scss` mentioned above; and one chunk over the default size threshold, see the note on chunking below. `go build ./...` and `go vet ./...` pass; `TestGetStyleResolvesBuiltAssets` passes against the Vite output; a search for `react-scripts`, `react-app-rewired`, `customize-cra` and `config-overrides` finds nothing outside the lockfile. The built binary was booted against sqlite3 and loaded in a browser: `/` renders with the client mounted (React's fiber container present on the root element), the module entry script and both entry stylesheets are fetched, and the console shows no errors. The same holds for `/tags`. The server-rendered `/` response itself carries the module entry script and both entry stylesheets, which is `GetStyle()` from step 1 finding them in the Vite output and `header.html` re-emitting them. The three `make check-ui` guards from #1567 (asset paths, non-default locale, plugin i18n order) are not in this PR; they arrive as step 3 and were run green against the equivalent tree there. ### Measurements Captured for #1567 at its head `d06b623`, against `main` at `3b9f137`, same machine, same Node and package manager versions, clean tree and clean install on both sides, five runs per timing metric. They are carried over rather than recaptured: the frontend build inputs here are the same as at that head, minus the step 3 nits and plus the newer locale files on `dev`. | Metric | Before | After | |---|---|---| | Cold production build | 19.87s | 4.77s | | Warm build | 8.75s | 4.87s | | Dev server time to ready | 7104ms | 359ms | | HMR latency | 421ms | 139ms | | Bundle JS, raw / gzip | 3477.60KB / 1198.75KB | 3010.49KB / 1035.57KB | | Bundle total, raw / gzip | 4290.39KB / 1652.24KB | 3763.97KB / 1474.34KB | | Direct dependencies | 75 | 65 | | Packages installed | 1578 | 689 | | Audit findings, critical/high/moderate/low | 4/78/63/13 | 1/50/38/6 | Five things worth stating rather than leaving to be inferred: - Cold and warm builds are within noise of each other, because the new build has no meaningful persistent cache to warm and now also runs a type check on every build, cold or warm alike. The old toolchain had a real warm cache, which is why its two figures differ so much. The comparison to draw is cold against cold. - Dev server time-to-ready is wall clock on both sides, including process spawn. The new tool self-reports a much smaller number that excludes that, and using it would compare two different quantities. - Chunk boundaries differ structurally between the two bundlers, so the bundle rows compare total shipped bytes rather than like-for-like chunks. - HMR latency was measured once, at commit 7200ca3 on the #1567 branch, on both toolchains, and carried forward from there: none of the commits since touch the hot-update path. - The bundle totals include the bootstrap-icons font files the first migration pass had silently dropped, and the cold build includes the restored type check. Both are named costs of parity fixes, already folded into the deltas. ### Not included This PR adds no CI job for the frontend. The project runs no frontend job today, and a build on every push is a cost a maintainer should choose to take on, not one this migration should impose. One constraint carries forward for whoever wires that job later: a bare `go test ./...` reports ok while asserting nothing about the built asset paths, because `TestGetStyleResolvesBuiltAssets` skips when no frontend build is embedded. Any future CI job needs to build the frontend first. The dev server now binds to loopback only by default; reaching it from another device on the network needs an explicit `--host` flag. Create React App's SVG-as-component imports (`import { ReactComponent as X } from './x.svg'`) are not carried over to this configuration. Nothing in this codebase used them. The commits carry `(cherry picked from commit ...)` lines pointing at the branch behind #1567, where these changes were first reviewed and where the regression matrix (subdirectory deploy, OAuth callbacks, absolute CDN `public_url`) was run.
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
16 participants
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.
No description provided.