Skip to content

chore: adopt Vite+ native type-aware checking - #3420

Open
setchy wants to merge 1 commit into
mainfrom
chore/type-aware-linting
Open

setchy wants to merge 1 commit into
mainfrom
chore/type-aware-linting

Conversation

@setchy

@setchy setchy commented Oct 14, 2026

Copy link
Copy Markdown
Member

Summary

Switches type checking to Vite+'s native path and drives all resulting type-aware findings to zero.

  • Enable lint.options.typeAware/typeCheck so vp check runs format, lint, and TypeScript type checks in one command. pnpm check now matches CI, closing the local/CI gap.
  • Drop the separate vp exec tsc --noEmit step from test.yml: lint.yml already runs vp run check, and tests depends on lint in both ci.yml and release.yml, so type-checking runs exactly once with no gap.
  • No change to ci.yml.

Supersedes #3417, which added vp check && vp exec tsc --noEmit. Native checking is equivalent and avoids the redundant tsc run.

Findings fixed

Enabling typeAware also turns on the type-aware lint rules; all 182 findings are resolved:

  • unbound-method on test doubles (100) — scoped off for **/*.test.*, __mocks__/__helpers__; the mocks are typed as the real Electron/interface classes.
  • unbound-method in source (18) — logger.ts, registry.ts, notifications.ts, tailwind.config.mts.
  • no-floating-promises (25), await-thenable (22) — void / removed redundant awaits.
  • no-misused-spread (11) — new shared mergeHeaders helper; plain-object test events.
  • no-base-to-string (3), plus LogoIcon.size made optional, the IpcHandler type, and graphql-config declared as a direct devDependency.

Notes for review

  • Five await expect.element(...) calls carry an inline typescript/await-thenable disable: Vitest types the matcher return as HTMLElement | null while the runtime returns a poll promise, so the await is required for retry semantics.
  • The native checker has a wider scope than tsc (it also checks root config files such as graphql.config.ts and tailwind.config.mts), which is how the missing graphql-config surfaced.
  • Visual-regression tests were not run locally (they require the arm64 Playwright container); their changes are behaviour-preserving.

Verification

  • pnpm check — pass (format + lint + type check, 441 files)
  • pnpm test — 1901 tests passing

Enable lint.options.typeAware/typeCheck so `vp check` runs format, lint,
and TypeScript type checks natively, then drop the separate `tsc --noEmit`
step from the test workflow. Type checking now happens once, in the `lint`
job that `tests` already depends on.

Drive the type-aware findings to zero:
- scope `typescript/unbound-method` off for test doubles, which are typed
  as the real Electron/interface classes
- add `void`/remove redundant `await` for floating promises and
  non-thenable awaits; normalise `HeadersInit` merging via mergeHeaders
- declare the previously transitive `graphql-config` devDependency
@setchy
setchy requested a review from afonsojramos as a code owner October 14, 2026 12:20
@github-actions github-actions Bot added chore Miscellaneous tasks dependency Dependency updates labels Oct 14, 2026
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
79.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Miscellaneous tasks dependency Dependency updates

Development

Successfully merging this pull request may close these issues.

1 participant