Skip to content

Fix the six outstanding eslint problems - #197

Open
mressler wants to merge 1 commit into
mobxjs:masterfrom
diamondkinetics:fix/lint-errors
Open

mressler wants to merge 1 commit into
mobxjs:masterfrom
diamondkinetics:fix/lint-errors

Conversation

@mressler

@mressler mressler commented Sep 2, 2026

Copy link
Copy Markdown

yarn lint exits non-zero on master — 4 errors, 2 warnings. CI runs only yarn test, so nothing has been enforcing it.

src/api/createModelSchema.ts   55:7   warning  'x' is assigned a value but never used
src/core/deserialize.ts        64:76  error    Missing semicolon
src/core/deserialize.ts        67:41  error    Missing semicolon
src/core/serialize.ts          10:31  error    Don't use `Function` as a type
src/core/serialize.ts          59:16  error    This assertion is unnecessary
src/utils/utils.ts             29:27  warning  'x' is defined but never used
✖ 6 problems (4 errors, 2 warnings)

The five easy ones

  • createModelSchema.ts — deletes const x: Clazz<any> = Object;, dead module-scope debris sitting after the function's closing brace.
  • utils.ts — ar.filter((x) => true) → ar.filter(() => true). Identical behaviour, including the sparse-array counting the adjacent comment refers to, since filter still skips holes.
  • deserialize.ts — two missing semicolons, applied with eslint --fix.
  • serialize.ts — drops an as Serialized<T>[] the compiler already infers.

The one that isn't obvious

Replacing Function in FunctionKeys<T>. The usual substitution is (...args: any[]) => any, but that is not equivalent inside a conditional type — it doesn't match class constructors, whereas Function does. Verified with tsc:

class C {}
type A = typeof C extends Function ? true : false;                       // true
type B = typeof C extends (...args: any[]) => any ? true : false;        // false  ← divergence
type D = typeof C extends ((...args: any[]) => any)
                        | (new (...args: any[]) => any) ? true : false;  // true

FunctionKeys<T> exists to exclude function-valued properties from Serialized<T>, so the naive fix would silently start including constructor-valued properties and recursing into them as objects. Since this is type-level only, no runtime test would have caught it.

So the PR introduces AnyFunction, unioning a call signature with a construct signature to preserve the original breadth:

type AnyFunction = ((...args: any[]) => any) | (new (...args: any[]) => any);

One thing deliberately not done

serialize.ts is not prettier-clean on master — tabs in the FunctionKeys block, and the implementation signature runs past printWidth: 100. I matched the file's local style instead of reformatting, so this diff stays two lines rather than a whole-file churn that buries the actual change. The four files I did touch are prettier-clean. Happy to send the reformat separately if you'd like it.

Verified

yarn lint exits 0. tsc -p tsconfig.json and tsc -p test/typescript/tsconfig.json both clean — the second matters, since it's what exercises Serialized<T> against real usage. yarn build clean. yarn test 70/70.

🤖 Generated with Claude Code

`yarn lint` exits non-zero on master with 4 errors and 2 warnings. CI only runs
`yarn test`, so nothing has been enforcing it.

  * createModelSchema.ts — deletes `const x: Clazz<any> = Object;`, dead
    module-scope debris sitting after the function body.
  * utils.ts — `ar.filter((x) => true)` becomes `ar.filter(() => true)`. Same
    behaviour, including the sparse-array counting the comment refers to, since
    `filter` still skips holes.
  * deserialize.ts — two missing semicolons, via `eslint --fix`.
  * serialize.ts — drops an `as Serialized<T>[]` that the compiler already
    infers.
  * serialize.ts — replaces `Function` in `FunctionKeys<T>`.

That last one is not the obvious substitution, so it is worth stating why.
The usual replacement for a banned `Function` is `(...args: any[]) => any`,
but that is NOT equivalent in a conditional type: it does not match class
constructors, whereas `Function` does. Verified with tsc — for a class `C`,
`typeof C extends Function` is true while `typeof C extends (...args: any[]) =>
any` is false. Since `FunctionKeys<T>` exists to exclude function-valued
properties from `Serialized<T>`, the naive fix would silently start including
constructor-valued properties and recursing into them. The introduced
`AnyFunction` unions a call signature with a construct signature, preserving
the original breadth.

serialize.ts is not prettier-clean on master (tabs in that block, and a
signature past printWidth). Left alone rather than reformatted, so this diff
stays reviewable; the four files I did touch are prettier-clean.

Verified: `yarn lint` exits 0, `tsc -p tsconfig.json` and
`tsc -p test/typescript/tsconfig.json` both clean, `yarn build` clean, and
`yarn test` passes 70/70.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant