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.
mcmire
force-pushed
the
update-sample-gas-price-service
branch
from
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.
1 participant
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
Medium risk due to a breaking service API/messenger surface change (new cache actions/events; removed
onRetry/onBreak/onDegraded) and a refactor of request execution to TanStack Query caching/deduplication that can alter retry/error behavior.Overview
SampleGasPricesServiceis migrated to extend@metamask/base-data-service’sBaseDataService(breaking), replacing the bespoke service-policy wrapper and removingonRetry/onBreak/onDegraded.fetchGasPricesnow runs throughfetchQuery(TanStack Query) to cache/deduplicate requests, adds optionalqueryClientConfigfor query-client tuning, and validates API responses via@metamask/superstruct.The service’s public messenger surface is expanded with cache support (
${serviceName}:invalidateQueriesaction pluscacheUpdated/granularCacheUpdatedevents), exports are updated, tests are rewritten around the new behavior, and new deps/TS project references are added.Written by Cursor Bugbot for commit 9c341e1. This will update automatically on new commits. Configure here.