Skip to content

Harden static analysis checks - #317

Merged
nicklemmon merged 8 commits into
mainfrom
type-safety-hardening
Sep 5, 2026
Merged

Harden static analysis checks#317
nicklemmon merged 8 commits into
mainfrom
type-safety-hardening

Conversation

@nicklemmon

Copy link
Copy Markdown
Owner

Summary

  • replace the broken ESLint configuration with Oxlint type-aware linting
  • split strict TypeScript verification into no-emit app, tooling, and script projects
  • remove unsafe dynamic asset and lookup casts; validate missing assets and invariant lookups explicitly
  • make QA read-only and add the implementation plan

Validation

  • npm run qa
  • 13 test files passed
  • 136 tests passed

@vercel

vercel Bot commented Sep 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
react-deckbuilder Ready Ready Preview Sep 5, 2026 12:12pm UTC

Comment thread src/types/cards.ts
stats: {
attack: number
}
// TODO: Can this be removed?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Hmm, why did I leave this comment.

Comment thread src/types/cards.ts Outdated
Comment on lines +14 to +17
align?: 'left' | 'right' | undefined
artwork?: string | undefined
status?: 'disabled' | 'in-play' | 'idle' | undefined
orientation?: 'face-up' | 'face-down' | undefined

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Having both optional and undefined at the same time doesn't seem ideal

Comment thread src/app.tsx
onCreate={(formData) => {
send({
type: 'CREATE_CHARACTER',
// @ts-expect-error

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Nice deletion!

Comment thread package.json
Comment on lines 17 to 23
@@ -23,7 +29,6 @@
"@xstate/react": "6.1.0",

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Why get rid of concurrently? Isn't this faster?

Comment thread src/components/avatar.tsx Outdated

/** Overlays the avatar and applies a flash when healing */
function HealingFlash({ onAnimationComplete }: { onAnimationComplete?: () => void }) {
function HealingFlash({ onAnimationComplete }: { onAnimationComplete?: (() => void) | undefined }) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Again with the optional + undefined - can we not do that? One or the other should be fine. It makes things much more complex.

Comment thread src/components/card.tsx Outdated
Comment on lines +22 to +24
isStacked?: boolean | undefined
onClick?: (() => void) | undefined
className?: string | undefined

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Again, the optional and undefined typing is not great.

Comment on lines +28 to +35
const characterClass = formData.get('characterClass')
const characterName = formData.get('characterName')
const characterPortrait = formData.get('characterPortrait')
if (
typeof characterClass !== 'string' ||
typeof characterName !== 'string' ||
typeof characterPortrait !== 'string'
)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Do we have zod available to validate these things? Can we add it?

Should that validation be part of the state machine?

Comment thread src/helpers/rng.ts
return Math.floor(Math.random() * maxExclusive)
}

pick<T>(xs: readonly T[]): T {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This method is defined twice.

And each method could use some JSDoc descriptions.

nicklemmon and others added 7 commits September 5, 2026 07:54
requireAsset was copy-pasted identically across cards.ts, item.ts, and
monsters.ts; moved it into vite.ts alongside the other glob helpers.
The css[key] ?? '' workaround for noUncheckedIndexedAccess was also
repeated ad hoc across ~10 components; added cx()/cssClass() in
helpers/css.ts to centralize the safe lookup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GzLmUWBV6CUEVjoDuTZkAF
@nicklemmon
nicklemmon merged commit 1840865 into main Sep 5, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant