Skip to content
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

[ppom-validator] Replace use of any with proper types (non-test files only) #144

Closed
MajorLift opened this issue Jan 30, 2024 · 0 comments · Fixed by #89
Closed

[ppom-validator] Replace use of any with proper types (non-test files only) #144

MajorLift opened this issue Jan 30, 2024 · 0 comments · Fixed by #89
Assignees

Comments

@MajorLift
Copy link
Contributor

MajorLift commented Jan 30, 2024

At the time of writing, there were 27 instances of any.

@MajorLift MajorLift self-assigned this Jan 30, 2024
MajorLift added a commit that referenced this issue Feb 15, 2024
## Explanation

Reduces repo-wide `any` count to 1 (excluding test files).

- Types `provider` variables using the `Provider` type from `@metamask/network-controller`.
  - The `Provider` type is now safe to use, with its recent alignment issues resolved as of MetaMask/eth-query#21.
  - See also MetaMask/core#2028).
- Fixes JSON-RPC typing for provider requests.
  - Installs `@metamask/utils` as a dependency.
- Types `PPOM` variables with provisional skeleton type. 
- Fixes for controller-messenger typing.
- Fixes for misc. `any` usage

## References

- Closes #144

## Changelog

### Changed
- Add `@metamask/utils` ^8.3.0 as a dependency. ([#89](#89))

### Removed
- **BREAKING:** `NetworkControllerStateChangeEvent` is removed from the `PPOMControllerEvents` union type. ([#89](#89))

### Fixed
- **BREAKING**: `PPOMController` class constructor option types are narrowed from `any`. ([#89](#89))
  - The constructor expects `provider` to be the `Provider` type from `@metamask/network-controller` (equivalent to `SafeEventEmitterProvider` from `@metamask/eth-json-rpc-provider`).
  - The constructor expects `onPreferencesChange` to be `(callback: (preferencesState: { securityAlertsEnabled: boolean } & Record<string, Json>) => void) => void`.
- **BREAKING:** The `UsePPOM` type replaces `any` with the `PPOM` type in its definition. ([#89](#89))
- **BREAKING:** When a `PPOM` class instance makes JSON-RPC requests to the provider, the `params` type is now widened from `Record<string, unknown>` to `JsonRpcParams`, and the response type is narrowed from `any` to `JsonRpcSuccess<Json>`. ([#89](#89))

## Commits

* Linter fixes

* Install `@metamask/utils` as dependency

* Add entries to depcheck ignore to fix yarn depcheck error

* Fix JSON RPC types for provider requests

* Fix controller-messenger typing

* Explicitly enumerate package-level exports from `ppom-controller`

* Type `provider` variables with `SafeEventEmitterProvider` from network-controller

* Type `ppom` variables with provisional skeleton type

* Fix various `any` usage and typos

* Add TODO for fixing `any`

* Improve error response typing for `#jsonRpcRequest`

* Replace null check for `oldestChainId` with non-null assertion

* Replace null check for `this.#ppom` with non-null assertion

* Replace unnecessary null fallback with non-null assertion

* Linter fix

* Record CHANGELOG

* Remove changelog entries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant