Repository navigation
Do not pad binding elements that the parameter's type already types - #64671
dev (Konan69) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Nested contextual bindings can remain implicitly any while their diagnostic is suppressed.
1 open finding
What changed in this PR
Prevents false TS7031 diagnostics for typed binding elements with default object literals.
Changes:
- Gates implicit-any reporting on root parameter typing.
- Adds contextual and annotated regression cases with baselines.
| File | Description |
|---|---|
tsc/internal/checker/checker.go |
Adjusts implicit-any reporting. |
tsc/testdata/tests/cases/compiler/objectBindingPatternDefaultMissingElements.ts |
Adds regression cases. |
tsc/testdata/baselines/reference/compiler/objectBindingPatternDefaultMissingElements.types |
Updates type baseline. |
tsc/testdata/baselines/reference/compiler/objectBindingPatternDefaultMissingElements.symbols |
Updates symbol baseline. |
tsc/testdata/baselines/reference/compiler/objectBindingPatternDefaultMissingElements.errors.txt |
Updates diagnostic baseline. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| // gets its binding element types from the annotation and a contextually typed one from the | ||
| // contextual signature, so the implicit any error would contradict the element's own type. | ||
| root := ast.GetRootDeclaration(pattern.Parent) | ||
| reportErrors := !(ast.IsParameterDeclaration(root) && (root.Type() != nil || c.getContextuallyTypedParameterType(root) != nil)) |
There was a problem hiding this comment.
Good catch, that was right. The new commit resolves each element through the annotation or contextual type and skips the padding for it, so the nested case is boolean | undefined again, and the test now asserts the types.
padObjectLiteralType pads every binding element missing from a parameter's
default object literal with the element's own type, which is implicitly any
when the element has no initializer. For an annotated or contextually typed
parameter that replaces the type the annotation or the contextual signature
gives the element, and reports TS7031 on an element that has a type:
type Options = (options?: { recursive?: boolean }) => void;
const f: Options = ({ recursive } = {}) => {};
// ~~~~~~~~~ TS7031, but recursive is boolean | undefined
Resolve the type the pattern destructures from the annotation or the
contextual signature, through nested patterns, and leave out of the padding
each element that type has. Elements it does not have are padded and reported
as before.
c435387 to
42826db
Compare
|
dev (@Konan69) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
## Human comments ## What was wrong Typechecking ran on TypeScript 7.0.2 (`typescript-7` → `npm:typescript@^7.0.2`). [tsrs](https://blog.cloudflare.com/typescript-rust-port), the Rust port of the TypeScript 7 compiler, is roughly twice as fast and uses half the memory on bb (numbers below). This PR moves bb's typechecking to it. tsrs 0.18.0 is a faithful port of TypeScript `7.1.0-dev.20260930.4` and prints the same diagnostics. Moving to it therefore also moves bb onto a 7.1 nightly, which brings two upstream regressions. tsrs also has no Windows binary yet: 1. **TS5115 on zod's `z.json()`** ("Instantiations of the following types appear infinitely circular: 'IsOptionalIn', '$ZodTypeInternals', '$InferZodRecordInput'"). Reported as microsoft/TypeScript#64605. The first nightly that fails is `7.1.0-dev.20260929.1`, after microsoft/TypeScript#64372. 2. **TS7031 on a contextually typed destructured parameter with `= {}`.** Reported as microsoft/TypeScript#64431, with fixes proposed in microsoft/TypeScript#64440 and microsoft/TypeScript#64671. The first nightly that fails is `7.1.0-dev.20260923.1`. 3. **No `win32-x64` package through 0.18.0.** tsrs's `npm/README.md` says its release workflow now builds one. Intel Macs remain unsupported. ## What changed - **tsrs replaces TypeScript 7 for typechecking.** Every `package.json` that listed `typescript-7` now lists `tsrs` pinned to `0.18.0-ts7.1.0-dev.20260929` (exact pin because the releases are experimental), and every `typecheck` script runs `tsrs --noEmit`. `typescript` (the `@typescript/typescript6` JS API) stays for tooling that imports it. - **Workaround for regression 1.** Core code uses `jsonValueSchema`/`jsonObjectSchema` from `@bb/domain` instead of `z.json()`: `apps/desktop` (CDP + capture, plus two tests) and `packages/desktop-contract`. Forkable plugins can't import `@bb/domain`, so account-pool, bb-account, connect, mcp and workflows each define the same `z.lazy` schema, typed with the SDK's `JsonValue`. That schema has the same structure as zod's `z.json()`, so runtime validation is unchanged. The only remaining `z.json()` calls are in two tests that don't trigger the error. - **Workaround for regression 2.** The abort-signal mock in `system-config-atoms.local-access.test.ts` annotates its parameter. - **Windows.** The `windows-foundation` job now runs `lint test` without `typecheck`. The Windows typecheck steps in `build-desktop.yml` and the `nightly-desktop-windows` job are removed. `plugin-authoring-doc-examples.test.ts` now launches tsrs and skips on win32. Types don't depend on the OS, and the Linux `static-checks` job still typechecks every package. These can come back once tsrs publishes `win32-x64`. - Left alone: the plugin scaffold's `typescript` dependency for user plugins, and the bb-app tarball smoke test's `npx typescript` consumer check. Both stand in for what plugin authors and SDK users run. ## How you verified - `pnpm exec turbo run typecheck --force --continue` on tsrs: 105/105 pass. Before the workarounds, 7 failed, with 16 TS5115 errors and 1 TS7031. - Every changed package also passes `tsc --noEmit` on TypeScript 7.0.2. - Both regressions reproduce on tsc `7.1.0-dev.20260930.4` and today's `7.1.0-dev.20261020.3`, and pass on 7.0.2. I found the first failing nightly for each by trying successive builds. - `turbo run lint test` for `@bb/desktop`, `@bb/desktop-contract`, account-pool, bb-account, connect, bb--mcp and workflows: 16/16 pass. The doc-examples test passes with tsrs, and the system-config atoms test passes (12/12). - Speed, same machine, warm, `--noEmit`, best of two runs: | package | tsc (TS 7.0.2) | tsrs | |---|---|---| | `apps/server` | 11.3s, 3.3 GB | 5.5s, 1.6 GB | | `apps/app` | 11.4s, 3.4 GB | 6.4s, 1.7 GB | - `pnpm install --frozen-lockfile` is clean, and `oxfmt --check` passes. > AGENT GENERATED --------- Co-authored-by: Sawyer <kirbyhood@gmail.com>

Fixes #64431
#64043 made
padObjectLiteralTypepad every binding element missing from a parameter's default object literal, using the element's own type, which is implicitlyanywhen the element has no initializer. For an annotated or contextually typed parameter this reports TS7031 on an element that already has a type, and for nested patterns it also replaces that type withany:Change
padObjectLiteralTypenow resolves the type the pattern destructures from the parameter's annotation or its contextual signature, walking nested object patterns by property name, and does not pad an element that this type has. Elements the type does not have are padded and reported exactly as before, so a parameter with no type anywhere keeps its TS7031, which is what #64043 intended. The lookup (getDeclaredTypeOfParameterBindingPattern) never consults an initializer.The first version of this PR only suppressed the diagnostic. The review pointed out that a nested contextually typed element then stayed
anysilently; that was right, and this version fixes the type as well as the diagnostic.Relation to #64440
I saw the draft in #64440 after starting this. It skips the padding only when every element of the pattern resolves through the contextual type. This PR decides per element, which differs in one case I could find:
knownunknownnumber | undefinedOn every other case below the two give the same result. Please close this if you prefer to finish #64440.
Tests
Added to
objectBindingPatternDefaultMissingElements, each with an assignment tostringso the errors baseline shows the element's type:= {}default in an annotated parameter (regression from #64043) #64431.go test ./internal/testrunner -run TestLocalandgo test ./internal/checker/...pass; the only baselines that change are that test's, and its 5 intended TS7031 errors are unchanged. I have not runhereby lintor the submodule tests.How we found it
We were evaluating type checkers and build tooling for a large TypeScript monorepo (an AI product: 17 projects, Effect 4). A checker that tracks current
mainreported 3 lines that7.0.2does not, all this error, on an object literal implementing an SDK interface (mkdir: (path, { recursive } = {}) => ...). Bisecting the nightlies gave7.1.0-dev.20260922.1as the last good and7.1.0-dev.20260923.1as the first bad;6.0.3and7.0.2accept the code.For reference, the whole-repo check on that monorepo, cold, 17 projects with declarations, on a 16-core machine:
tsc7.0.2,--checkers 1tsc7.0.2, default checkersAI disclosure
This patch was written with an AI coding tool. I have read it, I understand it, and I will handle the review myself.