Skip to content

chore: increase eslint/typescript strictness - #75

Open
robsimmons wants to merge 3 commits into
push-xmuqwrsvvrurfrom
push-xtywyksptyyw
Open

chore: increase eslint/typescript strictness#75
robsimmons wants to merge 3 commits into
push-xmuqwrsvvrurfrom
push-xtywyksptyyw

Conversation

@robsimmons

@robsimmons robsimmons commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Changes in #69, the first experiment at adding linting rules, seemed like they were dominated by adding ! annotations everywhere. Wanted to try to do something cleaner and higher-signal, though I did end up adding one noisy rule, typescript-eslint/consistent-type-imports, which insists on type imports being marked with type.

@robsimmons
robsimmons changed the base branch from main to push-xmuqwrsvvrur August 7, 2026 19:23
@robsimmons
robsimmons marked this pull request as draft August 7, 2026 19:23
@robsimmons
robsimmons force-pushed the push-xtywyksptyyw branch 2 times, most recently from c735f97 to b2955b2 Compare August 7, 2026 19:38
@robsimmons
robsimmons force-pushed the push-xtywyksptyyw branch 7 times, most recently from c41627e to 0623c8a Compare August 7, 2026 20:06
Comment thread src/app/setup/page.tsx
<div className='setup-progress-label'>
{phase === 'seeding' && <span className='setup-spinner' />}
<span>{progress.label || 'Starting...'}</span>
</div>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The linter noticed that this was redundant, there's an external check that the phase is seeding or error already.

Comment thread eslint.config.mjs
import prettier from 'eslint-config-prettier/flat'
import noRelativeImportPaths from 'eslint-plugin-no-relative-import-paths'
import simpleImportSort from 'eslint-plugin-simple-import-sort'
import unusedImports from 'eslint-plugin-unused-imports'

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I added unusedImports was added a while back so that prettier wouldn't delete unused imports, which made my workflow better. But typescript-eslint/no-unused-vars had a hidden option for removing unused imports that serves the same purpose without the dependency.

@robsimmons
robsimmons requested a review from Vtec234 August 7, 2026 20:15
@robsimmons
robsimmons marked this pull request as ready for review August 7, 2026 20:15
Comment thread src/lib/server/vscodeServer.ts Outdated
Comment thread eslint.config.mjs
},
rules: {
'@typescript-eslint/no-unused-vars': 'off',
'no-unused-vars': ['error', { args: 'none', caughtErrors: 'none' }],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
'no-unused-vars': ['error', { args: 'none', caughtErrors: 'none' }],
'no-unused-vars': ['error', { argsIgnorePattern: '^_', caughtErrors: 'none' }],

Would this be too disruptive? This specific lint seems useful.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not too disruptive and I changed it — but I actually changed it in @typescript-eslint/no-unused-vars, which seems like a better place to do it.

The relationship between @typescript-eslint/no-unused-vars and no-unused-vars is completely confusing to me though I think I once understood it.

FWIW, the reason I have this set to none in my person configurations is that

  1. You still get the grayed-out unused indication in vscode
  2. You don't get _arg stuff bleeding into interfaces for public functions that, for whatever reason, ignore their arguments. (This can happen when you change an implementation but want to leave the API stable, for example.)

@Vtec234 Vtec234 Aug 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One advantage of eslint over VSCode UI features is that CI would catch typos here. Your second point about backwards compat makes sense, though. Up to you what to do here!

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.

2 participants