-
Notifications
You must be signed in to change notification settings - Fork 43
refactor: Enable noImplicitOverride on the remaining non-generated code
#3304
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors TypeScript configuration across multiple packages to enable stricter type checking by setting noImplicitOverride to true for non-generated code. The refactoring isolates auto-generated protobuf code into separate configuration files with relaxed type checking, while enforcing stricter typing in hand-written source code.
Key changes:
- Introduced
tsconfig.generated.jsonfiles in 5 packages (autocertifier-client, dht, proto-rpc, sdk, trackerless-network) to isolate generated code withnoImplicitOverride: false - Removed
noImplicitOverride: falsefrom maintsconfig.jsonandtsconfig.jest.jsonfiles to enable stricter override checking - Updated build scripts to use
tsc -bfor proper project reference support
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/trackerless-network/tsconfig.json | Removed noImplicitOverride: false, excluded generated code, added reference to tsconfig.generated.json |
| packages/trackerless-network/tsconfig.jest.json | Removed compiler options override, added reference to tsconfig.generated.json |
| packages/trackerless-network/tsconfig.generated.json | New file isolating generated code with relaxed type checking |
| packages/trackerless-network/package.json | Updated check script to use tsc -b for project references |
| packages/sdk/tsconfig.json | Removed noImplicitOverride: false, excluded src/generated, added reference to tsconfig.generated.json |
| packages/sdk/tsconfig.jest.json | Removed noImplicitOverride: false, excluded src/generated, added reference to tsconfig.generated.json |
| packages/sdk/tsconfig.generated.json | New file isolating generated code with relaxed type checking |
| packages/sdk/src/identity/EthereumKeyPairIdentity.ts | Added override keyword to getTransactionSigner method |
| packages/sdk/package.json | Updated check script to use tsc -b for project references |
| packages/proto-rpc/tsconfig.json | Removed noImplicitOverride: false, excluded generated from include, added reference to tsconfig.generated.json |
| packages/proto-rpc/tsconfig.jest.json | Removed compiler options override, added reference to tsconfig.generated.json |
| packages/proto-rpc/tsconfig.generated.json | New file isolating generated code with relaxed type checking |
| packages/proto-rpc/package.json | Updated check script to use tsc -b for project references |
| packages/dht/tsconfig.json | Removed noImplicitOverride: false, excluded generated code, added reference to tsconfig.generated.json |
| packages/dht/tsconfig.jest.json | Removed compiler options override, added reference to tsconfig.generated.json |
| packages/dht/tsconfig.generated.json | New file isolating generated code with relaxed type checking |
| packages/dht/test/utils/utils.ts | Added override keyword to stop methods in anonymous classes |
| packages/dht/test/unit/PeerManager.test.ts | Added override keyword to ping method in anonymous class |
| packages/dht/package.json | Updated check script to use tsc -b for project references |
| packages/autocertifier-client/tsconfig.json | Removed noImplicitOverride: false, excluded generated from include, added reference to tsconfig.generated.json |
| packages/autocertifier-client/tsconfig.generated.json | New file isolating generated code with relaxed type checking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
330c33c to
b3f9c28
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
4b49512 to
9f31c91
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ride-on-remaining-packages
This pull request refactors TypeScript configuration across multiple packages to improve handling of generated code and project references, and updates some package scripts for consistency. It also includes minor code improvements for clarity and correctness.
Note
Previously,
noImplicitOverridewas explicitly set tofalseacross the app. This was not a design choice, but a workaround: the auto-generated protobuf types were incompatible withnoImplicitOverride, and there was no proper isolation at the tsconfig level to scope that limitation. As a result, stricter typing guarantees were relaxed globally.Changes in this PR are the remaining step towards bringing the restriction back, making the types stricter again.
Changes
TypeScript configuration improvements:
tsconfig.generated.jsonfiles in several packages (such asautocertifier-client,dht,proto-rpc, andsdk) to isolate generated code and manage its compilation independently. These are now referenced in the main and test TypeScript configs. [1] [2] [3]tsconfig.jsonandtsconfig.jest.jsonfiles to removenoImplicitOverride: false(now set only in generated configs), exclude generated directories from main source includes, and add references to the new generated configs. [1] [2] [3] [4] [5] [6] [7] [8]Project reference and dependency cleanup:
tsconfig.jsonfiles to ensure correct dependency resolution and build order. [1] [2] [3] [4] [5]Package script updates:
checkscript in severalpackage.jsonfiles to usetsc -b(build mode) instead oftsc -p, aligning with project references and improving type checking. [1] [2] [3] [4]Minor code improvements:
overridekeyword to overridden methods in test and implementation files for better type safety and clarity. [1] [2] [3] [4]