diff --git a/.changeset/tricky-donuts-fly.md b/.changeset/tricky-donuts-fly.md new file mode 100644 index 000000000..c294d5bdc --- /dev/null +++ b/.changeset/tricky-donuts-fly.md @@ -0,0 +1,5 @@ +--- +"@callstack/repack": patch +--- + +script should be cached after successfully loaded diff --git a/packages/repack/src/modules/ScriptManager/ScriptManager.ts b/packages/repack/src/modules/ScriptManager/ScriptManager.ts index c76cfbb49..8c3b3b642 100644 --- a/packages/repack/src/modules/ScriptManager/ScriptManager.ts +++ b/packages/repack/src/modules/ScriptManager/ScriptManager.ts @@ -275,8 +275,6 @@ export class ScriptManager extends EventEmitter { // If it returns true, we need to fetch the script if (fetch) { script.locator.fetch = true; - this.cache[cacheKey] = script.getCacheData(); - await this.saveCache(); } this.emit('resolved', script.toObject()); @@ -288,12 +286,8 @@ export class ScriptManager extends EventEmitter { // If no custom shouldUpdateScript function was provided, we use the default behaviour if (!this.cache[cacheKey]) { script.locator.fetch = true; - this.cache[cacheKey] = script.getCacheData(); - await this.saveCache(); } else if (script.shouldRefetch(this.cache[cacheKey])) { script.locator.fetch = true; - this.cache[cacheKey] = script.getCacheData(); - await this.saveCache(); } this.emit('resolved', script.toObject()); @@ -308,6 +302,14 @@ export class ScriptManager extends EventEmitter { } } + private async updateCache(script: Script) { + if (script.locator.fetch) { + const cacheKey = script.locator.uniqueId; + this.cache[cacheKey] = script.getCacheData(); + await this.saveCache(); + } + } + /** * Resolves given script's location, downloads and executes it. * The execution of the code is handled internally by threading in React Native. @@ -344,6 +346,7 @@ export class ScriptManager extends EventEmitter { this.emit('loading', script.toObject()); await this.loadScriptWithRetry(scriptId, script.locator); this.emit('loaded', script.toObject()); + await this.updateCache(script); } catch (error) { const { code } = error as Error & { code: string }; this.handleError( @@ -352,9 +355,10 @@ export class ScriptManager extends EventEmitter { code ? `[${code}]` : '', script.toObject() ); + } finally { + // should delete script promise even script failed + delete this.scriptsPromises[uniqueId]; } - - delete this.scriptsPromises[uniqueId]; }; this.scriptsPromises[uniqueId] = loadProcess(); @@ -424,6 +428,7 @@ export class ScriptManager extends EventEmitter { try { this.emit('prefetching', script.toObject()); await this.nativeScriptManager.prefetchScript(scriptId, script.locator); + await this.updateCache(script); } catch (error) { const { code } = error as Error & { code: string }; this.handleError( @@ -432,9 +437,10 @@ export class ScriptManager extends EventEmitter { code ? `[${code}]` : '', script.toObject() ); + } finally { + // should delete script promise even script failed + delete this.scriptsPromises[uniqueId]; } - - delete this.scriptsPromises[uniqueId]; }; this.scriptsPromises[uniqueId] = loadProcess(); diff --git a/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts b/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts index 2a5161613..49e9458cb 100644 --- a/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts +++ b/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts @@ -183,6 +183,8 @@ describe('ScriptManagerAPI', () => { uniqueId: 'main_src_App_js', }); + await ScriptManager.shared.loadScript('src_App_js', 'main'); + const { locator: { fetch }, } = await ScriptManager.shared.resolveScript('src_App_js', 'main'); @@ -435,6 +437,7 @@ describe('ScriptManagerAPI', () => { 'src_App_js', 'main' ); + await ScriptManager.shared.loadScript('src_App_js', 'main'); expect(script1.locator.fetch).toBe(false); ScriptManager.shared.removeAllResolvers(); @@ -454,6 +457,7 @@ describe('ScriptManagerAPI', () => { 'src_App_js', 'main' ); + await ScriptManager.shared.loadScript('src_App_js', 'main'); expect(script2.locator.fetch).toBe(true); ScriptManager.shared.removeAllResolvers(); @@ -473,6 +477,7 @@ describe('ScriptManagerAPI', () => { 'src_App_js', 'main' ); + await ScriptManager.shared.loadScript('src_App_js', 'main'); expect(script3.locator.fetch).toBe(true); ScriptManager.shared.removeAllResolvers(); @@ -496,6 +501,7 @@ describe('ScriptManagerAPI', () => { 'src_App_js', 'main' ); + await ScriptManager.shared.loadScript('src_App_js', 'main'); expect(script4.locator.fetch).toBe(true); ScriptManager.shared.removeAllResolvers(); @@ -519,6 +525,7 @@ describe('ScriptManagerAPI', () => { 'src_App_js', 'main' ); + await ScriptManager.shared.loadScript('src_App_js', 'main'); expect(script5.locator.fetch).toBe(false); ScriptManager.shared.removeAllResolvers(); @@ -590,7 +597,7 @@ describe('ScriptManagerAPI', () => { expect(NativeScriptManager.loadScript).toHaveBeenCalledTimes(1); expect(NativeScriptManager.loadScript).toHaveBeenCalledWith(scriptId, { absolute: false, - fetch: false, + fetch: true, method: 'GET', retry: 2, retryDelay: 100, @@ -646,7 +653,7 @@ describe('ScriptManagerAPI', () => { expect(NativeScriptManager.loadScript).toHaveBeenCalledTimes(3); expect(NativeScriptManager.loadScript).toHaveBeenCalledWith(scriptId, { absolute: false, - fetch: false, + fetch: true, method: 'GET', retry: 2, retryDelay: 100, @@ -761,7 +768,7 @@ describe('ScriptManagerAPI', () => { await ScriptManager.shared.loadScript('miniApp'); expect(loadingScriptIsFinished).toEqual(true); - spy.mockReset(); + spy.mockRestore(); }); @@ -803,7 +810,7 @@ describe('ScriptManagerAPI', () => { expect(loadingScriptIsFinished).toEqual(true); await ScriptManager.shared.loadScript('miniApp2'); expect(loadingScript2IsFinished).toEqual(true); - spy.mockReset(); + spy.mockRestore(); }); @@ -829,7 +836,41 @@ describe('ScriptManagerAPI', () => { await ScriptManager.shared.loadScript('miniApp'); expect(prefetchScriptIsFinished).toEqual(true); - spy.mockReset(); + + spy.mockRestore(); + }); + + it('should refetch failed script', async () => { + jest.useFakeTimers({ advanceTimers: true }); + const spy = jest.spyOn(NativeScriptManager, 'loadScript'); + + spy.mockRejectedValueOnce( + (_scriptId: string, _scriptConfig: NormalizedScriptLocator) => + Promise.reject({ code: 'NetworkFailed' }) + ); + + const cache = new FakeCache(); + ScriptManager.shared.setStorage(cache); + + ScriptManager.shared.addResolver(async (scriptId, _caller) => { + return { + url: Script.getRemoteURL(scriptId), + cache: true, + }; + }); + + await expect(async () => + ScriptManager.shared.loadScript('miniApp') + ).rejects.toThrow(); + + // //expected to cache be empty + expect(Object.keys(cache.data).length).toBe(0); + + await ScriptManager.shared.loadScript('miniApp'); + + // expected to fetch again + expect(spy.mock.lastCall?.[1].fetch).toBe(true); + spy.mockRestore(); }); });