diff --git a/packages/ramps-controller/CHANGELOG.md b/packages/ramps-controller/CHANGELOG.md index d116472fd66..8ec98315faf 100644 --- a/packages/ramps-controller/CHANGELOG.md +++ b/packages/ramps-controller/CHANGELOG.md @@ -7,11 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** Update state hydration to make `init()` idempotent and remove `hydrateState()` ([#8157](https://github.com/MetaMask/core/pull/8157)) + +### Removed + +- Remove `hydrateState()` — use `init()` as the single entry point for controller hydration + ## [11.0.0] ### Changed -- **BREAKING:** Replace `getWidgetUrl` with `getBuyWidgetData` (returns `BuyWidget | null`); add `addPrecreatedOrder` for custom-action ramp flows (e.g., PayPal) ([#8100](https://github.com/MetaMask/core/pull/8100)) +- **BREAKING:** Replace `getWidgetUrl` with `getBuyWidgetData` (returns `BuyWidget | null`); add `addPrecreatedOrder` for custom-action ramp flows (e.g., PayPal, Robinhood, Coinbase) ([#8100](https://github.com/MetaMask/core/pull/8100)) ## [10.2.0] diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 8b9c19c7cb1..daa7965089b 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -1667,98 +1667,139 @@ describe('RampsController', () => { }); }); }); - }); - describe('hydrateState', () => { - it('triggers fetching tokens and providers for user region', async () => { - await withController( - { - options: { - state: { - userRegion: createMockUserRegion('us-ca'), - }, + it('does not double-fetch when init() called twice concurrently', async () => { + await withController(async ({ controller, rootMessenger }) => { + let getCountriesCallCount = 0; + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => 'us-ca', + ); + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCallCount += 1; + return createMockCountries(); }, - }, - async ({ controller, rootMessenger }) => { - let tokensCalled = false; - let providersCalled = false; - - rootMessenger.registerActionHandler( - 'RampsService:getTokens', - async () => { - tokensCalled = true; - return { topTokens: [], allTokens: [] }; - }, - ); - rootMessenger.registerActionHandler( - 'RampsService:getProviders', - async () => { - providersCalled = true; - return { providers: [] }; - }, - ); - - controller.hydrateState(); - - await new Promise((resolve) => setTimeout(resolve, 10)); + ); + rootMessenger.registerActionHandler( + 'RampsService:getTokens', + async () => ({ topTokens: [], allTokens: [] }), + ); + rootMessenger.registerActionHandler( + 'RampsService:getProviders', + async () => ({ providers: [] }), + ); - expect(tokensCalled).toBe(true); - expect(providersCalled).toBe(true); - }, - ); + await Promise.all([controller.init(), controller.init()]); + expect(getCountriesCallCount).toBe(1); + }); }); - it('throws error when userRegion is not set', async () => { - await withController(async ({ controller }) => { - expect(() => controller.hydrateState()).toThrow( - 'Region is required. Cannot proceed without valid region information.', + it('returns immediately on second init() after first completes', async () => { + await withController(async ({ controller, rootMessenger }) => { + let getCountriesCallCount = 0; + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => 'us-ca', ); + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCallCount += 1; + return createMockCountries(); + }, + ); + rootMessenger.registerActionHandler( + 'RampsService:getTokens', + async () => ({ topTokens: [], allTokens: [] }), + ); + rootMessenger.registerActionHandler( + 'RampsService:getProviders', + async () => ({ providers: [] }), + ); + + await controller.init(); + await controller.init(); + expect(getCountriesCallCount).toBe(1); }); }); - it('calls getTokens and getProviders when hydrating even if state has data', async () => { - const existingProviders: Provider[] = [ - { - id: '/providers/test', - name: 'Test Provider', - environmentType: 'STAGING', - description: 'Test', - hqAddress: '123 Test St', - links: [], - logos: { light: '', dark: '', height: 24, width: 77 }, - }, - ]; + it('skips getCountries and geolocation when userRegion and countries exist', async () => { + let getCountriesCalled = false; + let getGeolocationCalled = false; await withController( { options: { state: { + countries: createResourceState(createMockCountries()), userRegion: createMockUserRegion('us-ca'), - providers: createResourceState(existingProviders, null), }, }, }, async ({ controller, rootMessenger }) => { - let providersCalled = false; + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCalled = true; + return createMockCountries(); + }, + ); + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => { + getGeolocationCalled = true; + return 'us-ca'; + }, + ); rootMessenger.registerActionHandler( 'RampsService:getTokens', async () => ({ topTokens: [], allTokens: [] }), ); rootMessenger.registerActionHandler( 'RampsService:getProviders', - async () => { - providersCalled = true; - return { providers: [] }; - }, + async () => ({ providers: [] }), ); - controller.hydrateState(); - - await new Promise((resolve) => setTimeout(resolve, 10)); + await controller.init(); - expect(providersCalled).toBe(true); + expect(getCountriesCalled).toBe(false); + expect(getGeolocationCalled).toBe(false); + expect(controller.state.userRegion?.regionCode).toBe('us-ca'); }, ); }); + + it('forceRefresh bypasses idempotency and re-runs full flow', async () => { + let getCountriesCallCount = 0; + await withController(async ({ controller, rootMessenger }) => { + rootMessenger.registerActionHandler( + 'RampsService:getGeolocation', + async () => 'us-ca', + ); + rootMessenger.registerActionHandler( + 'RampsService:getCountries', + async () => { + getCountriesCallCount += 1; + return createMockCountries(); + }, + ); + rootMessenger.registerActionHandler( + 'RampsService:getTokens', + async () => ({ topTokens: [], allTokens: [] }), + ); + rootMessenger.registerActionHandler( + 'RampsService:getProviders', + async () => ({ providers: [] }), + ); + + await controller.init(); + expect(getCountriesCallCount).toBe(1); + + await controller.init({ forceRefresh: true }); + expect(getCountriesCallCount).toBe(2); + }); + }); }); describe('setUserRegion', () => { diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index 95c2a13faf7..6f9e1b4a6ff 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -689,6 +689,8 @@ export class RampsController extends BaseController< #isPolling = false; + #initPromise: Promise | null = null; + /** * Clears the pending resource count map. Used only in tests to exercise the * defensive path when get() returns undefined in the finally block. @@ -1215,17 +1217,50 @@ export class RampsController extends BaseController< * Initializes the controller by fetching the user's region from geolocation. * This should be called once at app startup to set up the initial region. * - * If a userRegion already exists (from persistence or manual selection), - * this method will skip geolocation fetch and use the existing region. + * Idempotent: subsequent calls return the same promise unless forceRefresh is set. + * Skips getCountries when countries are already loaded; skips geolocation when + * userRegion already exists. * - * @param options - Options for cache behavior. + * @param options - Options for cache behavior. forceRefresh bypasses idempotency and re-runs the full flow. * @returns Promise that resolves when initialization is complete. */ async init(options?: ExecuteRequestOptions): Promise { - await this.getCountries(options); + if (!options?.forceRefresh && this.#initPromise !== null) { + return this.#initPromise; + } + + if (options?.forceRefresh) { + this.#initPromise = null; + } - let regionCode = this.state.userRegion?.regionCode; - regionCode ??= await this.messenger.call('RampsService:getGeolocation'); + const initPromise = this.#runInit(options).then( + () => undefined, + (error) => { + if (this.#initPromise === initPromise) { + this.#initPromise = null; + } + throw error; + }, + ); + this.#initPromise = initPromise; + return initPromise; + } + + async #runInit(options?: ExecuteRequestOptions): Promise { + const forceRefresh = options?.forceRefresh === true; + const hasCountries = this.state.countries.data.length > 0; + + if (forceRefresh || !hasCountries) { + await this.getCountries(options); + } + + let regionCode: string | undefined; + if (forceRefresh) { + regionCode = await this.messenger.call('RampsService:getGeolocation'); + } else { + regionCode = this.state.userRegion?.regionCode; + regionCode ??= await this.messenger.call('RampsService:getGeolocation'); + } if (!regionCode) { throw new Error( @@ -1236,13 +1271,6 @@ export class RampsController extends BaseController< await this.setUserRegion(regionCode, options); } - hydrateState(options?: ExecuteRequestOptions): void { - const regionCode = this.#requireRegion(); - - this.#fireAndForget(this.getTokens(regionCode, 'buy', options)); - this.#fireAndForget(this.getProviders(regionCode, options)); - } - /** * Fetches the list of supported countries. * The API returns countries with support information for both buy and sell actions.