chore: enable TypeScript strict mode#3112
chore: enable TypeScript strict mode#3112kyungseopk1m wants to merge 2 commits intofirebase:mainfrom
Conversation
Replace individual strict compiler options with "strict": true in tsconfig.json. This also enables strictPropertyInitialization, which required adding definite assignment assertions (!:) to properties initialized via Object.defineProperty helpers or Promise executors. Additionally fixes useUnknownInCatchVariables errors introduced by the new flag.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables strict type checking in the TypeScript configuration and updates the codebase to comply with these stricter rules. The changes involve adding definite assignment assertions, making certain properties optional, and explicitly casting error types in catch blocks across various modules. Additionally, the RemoteConfig implementation was refactored to centralize cache validation into a new helper method, supported by additional unit tests. Review feedback suggests using dynamic method names instead of hardcoded strings in error messages to improve maintainability.
| 'failed-precondition', | ||
| 'No Remote Config Server template in cache. Call load() before calling evaluate().'); | ||
| } | ||
| const cachedTemplate = this.getCachedTemplate('evaluate()'); |
There was a problem hiding this comment.
The method getCachedTemplate is called with 'evaluate()' as an argument. While this works, it is better to pass the method name dynamically using this.evaluate.name to avoid hardcoding the string and ensure it stays in sync if the method is renamed.
| * @returns JSON representation of the server template | ||
| */ | ||
| public toJSON(): ServerTemplateData { | ||
| return this.getCachedTemplate('toJSON()'); |
There was a problem hiding this comment.
Similar to the evaluate method, use this.toJSON.name instead of the hardcoded string 'toJSON()' to improve maintainability.
1 participant
Summary
Completes the long-standing TODO in
tsconfig.json:"strict": truestrictPropertyInitialization(previously commented out)TS2564errors using definite assignment assertions (!:) for properties initialized viaObject.defineProperty-based helpers (addReadonlyGetter,copyAttr) or Promise executor callbacksTS18046errors from newly enableduseUnknownInCatchVariableswithas Errorcasts in catch blocksServerTemplateImpl.cacheto use optional type (?:) with agetCachedTemplate()helper for proper runtime validation; adds corresponding testsChanges
tsconfig.json"strict": truesrc/**/*.ts(16 files)!:/?:/as Errorto resolve new strict errorstest/unit/remote-config/remote-config.spec.tstsc --noEmitpasses with zero errors after these changes.