Migrate SampleGasPriceService to BaseDataService#8343
Conversation
With the release of `@metamask/base-data-service`, the `SampleGasPricesService` class needs to be updated so that new data services that other engineers create follow the standards we want them to follow.
186aee8 to
896f3cc
Compare
| * @returns The root messenger. | ||
| */ | ||
| function getRootMessenger(): RootMessenger { | ||
| function createRootMessenger(): RootMessenger { |
There was a problem hiding this comment.
get* did not seem like an appropriate prefix for a factory function. The choice of prefix for factory functions was brought up in the past (by Erik I believe) and if I remember correctly we settled on create*.
| * {@link CockatielEvent}. | ||
| * @see {@link createServicePolicy} | ||
| */ | ||
| onRetry(listener: Parameters<ServicePolicy['onRetry']>[0]): IDisposable { |
There was a problem hiding this comment.
We probably should add these methods (or something like them) to BaseDataService, but regardless, I don't want to ask that engineers add them to their data service classes unless they really need to.
| ) { | ||
| return { low, average, high }; | ||
| } | ||
| if (!is(jsonResponse, GasPricesResponseStruct)) { |
There was a problem hiding this comment.
We might as well switch to using Superstruct for response validation. Most teams are shifting to using this anyway.
|
|
||
| ### Changed | ||
|
|
||
| - **BREAKING:** `SampleGasPricesService` now inherits from `BaseDataService` from `@metamask/base-data-service` ([#8343](https://github.com/MetaMask/core/pull/8343)) |
There was a problem hiding this comment.
This very well may not be breaking. I'll have to take a closer look at this, but I wanted to call it out in case it was.
| }, | ||
| ); | ||
|
|
||
| it('calls onDegraded listeners if the request takes longer than 5 seconds to resolve', async () => { |
There was a problem hiding this comment.
I removed these methods from the class so these tests are no longer necessary.
Explanation
With the release of
@metamask/base-data-service, theSampleGasPricesServiceclass needs to be updated so that new data services follow our best practices.References
(No ticket, but stumbled upon this while reviewing other PRs such as the
AuthenticatedUserStorageservice class: #8260)Checklist
Note
Medium Risk
Introduces breaking API changes (service inheritance and removed
onRetry/onBreak/onDegraded) and changes request behavior to be query-cached/deduplicated, which could affect consumers and error/retry semantics.Overview
Migrates
SampleGasPricesServiceto extend@metamask/base-data-service’sBaseDataService, switchingfetchGasPricesto usefetchQuery(TanStack Query) so API calls are cached and deduplicated and adding optionalqueryClientConfigfor cache/client tuning.Adds new messenger surface area for cache management/observability (
:invalidateQueriesaction pluscacheUpdatedevents), exports the new types fromsrc/index.ts, and updates tests accordingly while removing the previousonRetry/onBreak/onDegradedhooks. Dependency and TS project references are updated to include@metamask/base-data-service,@tanstack/query-core, and@metamask/superstruct(used for response validation).Written by Cursor Bugbot for commit 3a60272. This will update automatically on new commits. Configure here.