Skip to content

chore: increase eslint/typescript strictness #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
robsimmons merged 3 commits into from
Aug 9, 2026
Merged

chore: increase eslint/typescript strictness #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d89696c
chore: increase eslint/typescript strictness
robsimmons Aug 7, 2026
48a93b7
switch lint order
robsimmons Aug 7, 2026
41879f2
PR comment responses
robsimmons Aug 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion collab-server/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ server.httpServer.listen(socketPath, () => {
Object.defineProperty(server, 'httpURL', {
get: () => `http+unix:${socketPath}`,
})
server['showStartScreen']()
// Deliberate abstraction violation to call showStartScreen()
;(server as unknown as { showStartScreen(): void }).showStartScreen()

// No need to call `onListen` hooks here since we don't register any.
})
Expand Down
53 changes: 48 additions & 5 deletions eslint.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import eslint from '@eslint/js'
import { defineConfig, globalIgnores } from 'eslint/config'
import nextVitals from 'eslint-config-next/core-web-vitals'
import nextTs from 'eslint-config-next/typescript'
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'

@robsimmons robsimmons Aug 7, 2026

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.

import tseslint from 'typescript-eslint'

const eslintConfig = defineConfig([
...nextVitals,
Expand All @@ -25,15 +26,46 @@ const eslintConfig = defineConfig([
'vscode-workbench/src/vscode.proposed.*.d.ts',
]),
{
extends: [eslint.configs.recommended],
plugins: {
'simple-import-sort': simpleImportSort,
'unused-imports': unusedImports,
},
rules: {
'@typescript-eslint/no-unused-vars': 'off',
'no-unused-vars': ['error', { args: 'none', caughtErrors: 'none' }],

@Vtec234 Vtec234 Aug 8, 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.

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.

@robsimmons robsimmons Aug 9, 2026

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 •
edited
Loading

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!

'simple-import-sort/imports': 'warn',
'unused-imports/no-unused-imports': 'warn',
'unused-imports/no-unused-vars': ['warn', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }],
},
},
{
// Typescript rules
files: ['**/*.{mts,ts,tsx}'],
extends: tseslint.configs.recommendedTypeChecked,
languageOptions: { parserOptions: { projectService: true } },
rules: {
'@typescript-eslint/consistent-type-imports': ['warn', { fixStyle: 'inline-type-imports' }], // always 'import type'
'@typescript-eslint/no-base-to-string': 'off', // interacts badly with typing issues in yjs
'@typescript-eslint/no-confusing-void-expression': ['error', { ignoreArrowShorthand: true }], // allow (x) => console.log(x), ban const x = console.log(x)
'@typescript-eslint/no-misused-promises': [
'error',
{
// these exceptions reduce unnecessary friction with React stuff
checksVoidReturn: { arguments: false, attributes: false },
},
],
'@typescript-eslint/no-unnecessary-condition': 'off', // actively misleading until we turn the typescript option noUncheckedIndexedAccess on
'@typescript-eslint/no-unnecessary-type-assertion': 'off', // actively misleading until we turn the typescript option noUncheckedIndexedAccess on
'@typescript-eslint/no-unused-vars': [
'error',
{
argsIgnorePattern: '^_',
varsIgnorePattern: '^_',
caughtErrors: 'none',
enableAutofixRemoval: { imports: true },
},
],
'@typescript-eslint/no-unsafe-return': 'off', // noisy, temporarily disabled
'@typescript-eslint/require-await': 'off', // actively misleading in `'use server'` modules
'@typescript-eslint/restrict-template-expressions': 'off', // always allow `${x}` regardless of x's type
'@typescript-eslint/use-unknown-in-catch-callback-variable': 'error', // complements how strict works in typescript for chained promises
},
},
{
Expand All @@ -47,6 +79,17 @@ const eslintConfig = defineConfig([
],
},
},
{
// Test files may need to make use of the `any` type in a way we want to
// prevent in normal code.
files: ['**/*.{spec,test}.{ts,tsx}', '**/test/**'],
rules: {
'@typescript-eslint/no-unsafe-argument': 'off',
'@typescript-eslint/no-unsafe-assignment': 'off',
'@typescript-eslint/no-unsafe-member-access': 'off',
'@typescript-eslint/unbound-method': 'off',
},
},
])

export default eslintConfig
Loading
Loading