Skip to content

Enable strict TypeScript, add errorMessage helper#37292

Merged
silverwind merged 13 commits intogo-gitea:mainfrom
silverwind:strict
Apr 20, 2026
Merged

Enable strict TypeScript, add errorMessage helper#37292
silverwind merged 13 commits intogo-gitea:mainfrom
silverwind:strict

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 19, 2026

Enable full TypeScript strict mode and fix issues discovered during this refactor. Introduced a errorMessage helper function to cleanly extract a error messages from the unknown type.


This PR was written with the help of Claude Opus 4.7

Flip TypeScript's full `strict: true` on by removing the two remaining
overrides (`strictPropertyInitialization: false` and
`useUnknownInCatchVariables: false`) and fix the resulting errors:

- Class fields that are populated in a post-construction init path get a
  definite-assignment `!`. Fields with a natural empty default (`[]`)
  are initialized instead of asserted. Nullable fields default to
  `null`.
- Catch blocks annotate the caught variable as `any` so the existing
  `err.message` / `err.name` / `${err}` access patterns type-check.

Co-Authored-By: Claude (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 19, 2026
@silverwind silverwind requested a review from Copilot April 19, 2026 11:37
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

This PR enables full TypeScript strict mode across the frontend codebase by removing previously disabled strict sub-options and updating code to satisfy the stricter type checks.

Changes:

  • Enable stricter TS checks by removing strictPropertyInitialization and useUnknownInCatchVariables overrides from tsconfig.json.
  • Update various classes/components to satisfy strictPropertyInitialization (definite assignment assertions, default initializations).
  • Update multiple catch clauses to type the caught value as any to avoid unknown friction under strict mode.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
web_src/js/webcomponents/overflow-menu.ts Initialize/definitely-assign fields for strict property initialization; simplify overflowItems iteration.
web_src/js/utils/match.ts Type catch variable as any under strict mode.
web_src/js/markup/render-iframe.ts Type catch variable as any under strict mode.
web_src/js/markup/mermaid.ts Type catch variable as any under strict mode.
web_src/js/markup/math.ts Type catch variable as any under strict mode.
web_src/js/features/user-auth-webauthn.ts Type catch variables as any; adjust error reporting in one place.
web_src/js/features/repo-issue.ts Type catch variable as any under strict mode.
web_src/js/features/repo-issue-sidebar-combolist.ts Initialize initialValues and type catch variable as any.
web_src/js/features/repo-issue-list.ts Type catch variable as any under strict mode.
web_src/js/features/repo-issue-edit.ts Type catch variable as any under strict mode.
web_src/js/features/repo-diff.ts Type catch variable as any under strict mode.
web_src/js/features/repo-common.ts Type catch variable as any under strict mode.
web_src/js/features/imagediff.ts Definitely-assign fields for strict property initialization.
web_src/js/features/file-view.ts Type catch variable as any under strict mode.
web_src/js/features/dropzone.ts Type catch variable as any under strict mode.
web_src/js/features/comp/ComboMarkdownEditor.ts Definitely-assign / initialize fields for strict property initialization.
web_src/js/features/common-fetch-action.ts Type catch variable as any under strict mode.
web_src/js/features/citation.ts Type catch variable as any under strict mode.
web_src/js/features/admin/config.ts Type catch variable as any under strict mode.
web_src/js/components/RepoRecentCommits.vue Type catch variable as any under strict mode.
web_src/js/components/RepoContributors.vue Type catch variable as any under strict mode.
web_src/js/components/RepoCodeFrequency.vue Type catch variable as any under strict mode.
tsconfig.json Remove overrides that disabled strict sub-checks, thereby enforcing full strict behavior.

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

Comment thread web_src/js/features/user-auth-webauthn.ts Outdated
silverwind and others added 2 commits April 19, 2026 13:44
Replaces `catch (err: any)` with `catch (err)` (defaults to `unknown`
under strict mode) and moves the assumption to the use site via `as
Error` / `String(err)`. Runtime behavior is identical — assertions are
compile-time only — but the catch binding itself is no longer `any`,
forcing future code in the catch block to narrow.

For `webAuthnRegisterRequest`, use `err instanceof Error ? err.message
: String(err)` to preserve a fallback string when a non-Error is thrown
(addresses review feedback on go-gitea#37292).

Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com>
`String(err)` on a DOMException yields `"NotAllowedError: msg"` — the
name prefix was previously shown to users (via template-literal
coercion of the thrown value) and carries information they can act on
(NotAllowedError, SecurityError, etc.). The prior patch reduced it to
just `err.message`, losing that context. Use `String(err)` which
matches the pre-PR UI text exactly for Error/DOMException instances.

Co-Authored-By: Claude (Claude Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 19, 2026
setupTab() returns early when the container has no `.tabular.menu >
.item` elements, in which case `tabEditor` is never assigned. The
previous `!` assertion papered over this — `switchTabToEditor()` would
throw `Cannot read properties of undefined` on any markup without tabs.
Type the fields as optional so the compiler enforces the guard; the
call site now uses `?.click()`.

Co-Authored-By: Claude (Claude Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind marked this pull request as draft April 19, 2026 12:01
silverwind and others added 3 commits April 19, 2026 14:04
Add `errorMessage(err: unknown): string` centralizing the `(err as
Error).message` pattern that appeared at 7 catch sites. Runtime
behavior is identical — the helper performs the same compile-time
assertion.

Also widen `displayError(el, err)` in `markup/common.ts` from `Error`
to `unknown`, moving the assertion inside. The two callers in
`markup/math.ts` and `markup/mermaid.ts` no longer need their own
`as Error` cast.

Down from 11 `as Error` sites to 4 (the helper itself,
`displayError`'s body, and the `.name`/`.toString()` readers in
`common-fetch-action.ts` and `admin/config.ts` which aren't covered
by the helper).

Co-Authored-By: Claude (Claude Opus 4.7) <noreply@anthropic.com>
`errorMessage(err) || String(err)` matches the previous
`err.message || String(err)` exactly at runtime while dropping the
local `as Error` cast. One less ad-hoc assertion.

Co-Authored-By: Claude (Claude Opus 4.7) <noreply@anthropic.com>
Unifies all 8 use sites to the same semantics. Minor behavior
difference at Error instances with an empty `.message` (now returns
`'Error'` instead of `''`) — an edge case that doesn't occur for the
fetch / WebAuthn / explicit-throw errors these catches see in
practice, and displaying `'Error'` as a fallback is what
`displayError` already did.

Co-Authored-By: Claude (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

Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.


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

Comment thread web_src/js/features/repo-issue-list.ts
Comment thread web_src/js/utils/error.ts Outdated
@silverwind silverwind marked this pull request as ready for review April 19, 2026 12:20
Comment thread web_src/js/utils/error.ts Outdated
silverwind and others added 2 commits April 19, 2026 14:22
Signed-off-by: silverwind <me@silverwind.io>
Prefer the existing errorMessage() util over ad-hoc String(e) /
(err as Error).toString() so error objects display their .message
rather than a bare object stringification.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind requested a review from Copilot April 19, 2026 12:30
@silverwind silverwind changed the title Enable strict typescript Enable strict typescript, add errorMessage helper Apr 19, 2026
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 23 out of 24 changed files in this pull request and generated 1 comment.


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

Comment thread web_src/js/features/repo-settings-branches.ts Outdated
Co-locates it next to the other error-handling utilities and drops the
one-function utils/error.ts. Tree-shaking keeps it out of the IIFE
bootstrap chunk (byte-identical before/after).

Also converts repo-issue-list's `e.message` fallback to errorMessage,
and fixes a stray `:,` in the branch-priority toast noticed in review.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind changed the title Enable strict typescript, add errorMessage helper Enable strict TypeScript, add errorMessage helper Apr 19, 2026
@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 19, 2026
@lunny lunny added this to the 1.27.0 milestone Apr 19, 2026
@silverwind
Copy link
Copy Markdown
Member Author

Removed the note about the behaviour change, I see ! is right in this case, the crash condition is unreachable.

@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 20, 2026
@silverwind silverwind enabled auto-merge (squash) April 20, 2026 07:20
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 20, 2026
@silverwind silverwind merged commit f696009 into go-gitea:main Apr 20, 2026
26 checks passed
@silverwind silverwind deleted the strict branch April 20, 2026 07:22
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 20, 2026
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 20, 2026
…-lang

* origin/main:
  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)
  Use updated yaml fields for snapcraft (go-gitea#37318)
  Remove dead code identified by `deadcode` tool (go-gitea#37271)
  Enable strict TypeScript, add `errorMessage` helper (go-gitea#37292)
  Fix vite manifest update masking build errors (go-gitea#37279)
  bump snapcraft base (go-gitea#37301)

# Conflicts:
#	web_src/js/modules/errors.ts
silverwind added a commit to 6543-forks/gitea that referenced this pull request Apr 21, 2026
…n-better

* origin/main: (645 commits)
  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)
  Use updated yaml fields for snapcraft (go-gitea#37318)
  Remove dead code identified by `deadcode` tool (go-gitea#37271)
  Enable strict TypeScript, add `errorMessage` helper (go-gitea#37292)
  Fix vite manifest update masking build errors (go-gitea#37279)
  bump snapcraft base (go-gitea#37301)
  Add WebKit to e2e test matrix (go-gitea#37298)
  Don't add useless labels which will bother changelog generation (go-gitea#37267)
  Fix Repository transferring page (go-gitea#37277)
  ...

# Conflicts:
#	options/locale/locale_en-US.ini
#	templates/package/content/debian.tmpl
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. type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants