Skip to content

Commit

Permalink
[base-controller] Fix any usage in BaseControllerV1 (#3959)
Browse files Browse the repository at this point in the history
## Explanation

- For runtime property assignment, use `as unknown as` instead of `as
any`.
- Change the types for `BaseControllerV1` class fields `initialConfig`,
`initialState` from `C`, `S` to `Partial<C>`, `Partial<S>`.
- Initial user-supplied constructor options do not need to be complete
`C`, `S` objects, since `internal{Config,State}` will be populated with
`default{Config,State}`.
- For empty objects, prefer no type assertions or `as never` (`never` is
assignable to all types).
- Fix code written based on outdated TypeScript limitation.
- Generic spread expressions for object literals are supported by
TypeScript: microsoft/TypeScript#28234

## References

- Closes #3715 

## Changelog

###
[`@metamask/base-controller`](https://github.com/MetaMask/core/pull/3959/files#diff-a8212838da15b445582e5622bd4cc8195e4c52bcf87210af8074555f806706a9)

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
MajorLift authored Feb 27, 2024
1 parent 26ae591 commit eda5b9d
Showing 1 changed file with 14 additions and 17 deletions.
31 changes: 14 additions & 17 deletions packages/base-controller/src/BaseControllerV1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ export class BaseControllerV1<C extends BaseConfig, S extends BaseState> {
/**
* Default options used to configure this controller
*/
defaultConfig: C = {} as C;
defaultConfig: C = {} as never;

/**
* Default state set on this controller
*/
defaultState: S = {} as S;
defaultState: S = {} as never;

/**
* Determines if listeners are notified of state changes
Expand All @@ -59,9 +59,9 @@ export class BaseControllerV1<C extends BaseConfig, S extends BaseState> {
*/
name = 'BaseController';

private readonly initialConfig: C;
private readonly initialConfig: Partial<C>;

private readonly initialState: S;
private readonly initialState: Partial<S>;

private internalConfig: C = this.defaultConfig;

Expand All @@ -76,10 +76,9 @@ export class BaseControllerV1<C extends BaseConfig, S extends BaseState> {
* @param config - Initial options used to configure this controller.
* @param state - Initial state to set on this controller.
*/
constructor(config: Partial<C> = {} as C, state: Partial<S> = {} as S) {
// Use assign since generics can't be spread: https://git.io/vpRhY
this.initialState = state as S;
this.initialConfig = config as C;
constructor(config: Partial<C> = {}, state: Partial<S> = {}) {
this.initialState = state;
this.initialConfig = config;
}

/**
Expand Down Expand Up @@ -128,21 +127,19 @@ export class BaseControllerV1<C extends BaseConfig, S extends BaseState> {
? (config as C)
: Object.assign(this.internalConfig, config);

for (const [key, value] of Object.entries(this.internalConfig)) {
for (const key of Object.keys(this.internalConfig) as (keyof C)[]) {
const value = this.internalConfig[key];
if (value !== undefined) {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(this as any)[key] = value;
(this as unknown as C)[key] = value;
}
}
} else {
for (const key of Object.keys(config) as (keyof C)[]) {
/* istanbul ignore else */
if (typeof this.internalConfig[key] !== 'undefined') {
this.internalConfig[key] = (config as C)[key];
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(this as any)[key] = config[key];
if (this.internalConfig[key] !== undefined) {
const value = (config as C)[key];
this.internalConfig[key] = value;
(this as unknown as C)[key] = value;
}
}
}
Expand Down

0 comments on commit eda5b9d

Please sign in to comment.