diff --git a/.changeset/olive-singers-attack.md b/.changeset/olive-singers-attack.md new file mode 100644 index 000000000..7cba91855 --- /dev/null +++ b/.changeset/olive-singers-attack.md @@ -0,0 +1,6 @@ +--- +'@callstack/repack': patch +--- + +Prevent to loadScript which is already is loading +issue: https://github.com/callstack/repack/issues/749 diff --git a/packages/repack/src/modules/ScriptManager/ScriptManager.ts b/packages/repack/src/modules/ScriptManager/ScriptManager.ts index 744e40af1..c76cfbb49 100644 --- a/packages/repack/src/modules/ScriptManager/ScriptManager.ts +++ b/packages/repack/src/modules/ScriptManager/ScriptManager.ts @@ -12,6 +12,11 @@ type Cache = Record< Pick >; +type ScriptsPromises = Record< + string, + (Promise & { isPrefetch?: true }) | undefined +>; + const CACHE_NAME = 'Repack.ScriptManager.Cache'; const CACHE_VERSION = 'v4'; const CACHE_ENV = __DEV__ ? 'debug' : 'release'; @@ -100,6 +105,7 @@ export class ScriptManager extends EventEmitter { } protected cache: Cache = {}; + protected scriptsPromises: ScriptsPromises = {}; protected cacheInitialized = false; protected resolvers: [number, ScriptLocatorResolver][] = []; protected storage?: StorageApi; @@ -320,21 +326,39 @@ export class ScriptManager extends EventEmitter { caller?: string, webpackContext = getWebpackContext() ) { - const script = await this.resolveScript(scriptId, caller, webpackContext); - - try { - this.emit('loading', script.toObject()); - await this.loadScriptWithRetry(scriptId, script.locator); - this.emit('loaded', script.toObject()); - } catch (error) { - const { code } = error as Error & { code: string }; - this.handleError( - error, - '[ScriptManager] Failed to load script:', - code ? `[${code}]` : '', - script.toObject() - ); + const uniqueId = Script.getScriptUniqueId(scriptId, caller); + if (this.scriptsPromises[uniqueId]) { + const { isPrefetch } = this.scriptsPromises[uniqueId]; + + // prefetch is not execute the script so we need to run loadScript if promise is for prefetch + if (isPrefetch) { + await this.scriptsPromises[uniqueId]; + } else { + return this.scriptsPromises[uniqueId]; + } } + const loadProcess = async () => { + const script = await this.resolveScript(scriptId, caller, webpackContext); + + try { + this.emit('loading', script.toObject()); + await this.loadScriptWithRetry(scriptId, script.locator); + this.emit('loaded', script.toObject()); + } catch (error) { + const { code } = error as Error & { code: string }; + this.handleError( + error, + '[ScriptManager] Failed to load script:', + code ? `[${code}]` : '', + script.toObject() + ); + } + + delete this.scriptsPromises[uniqueId]; + }; + + this.scriptsPromises[uniqueId] = loadProcess(); + return this.scriptsPromises[uniqueId]; } /** @@ -390,20 +414,32 @@ export class ScriptManager extends EventEmitter { caller?: string, webpackContext = getWebpackContext() ) { - const script = await this.resolveScript(scriptId, caller, webpackContext); - - try { - this.emit('prefetching', script.toObject()); - await this.nativeScriptManager.prefetchScript(scriptId, script.locator); - } catch (error) { - const { code } = error as Error & { code: string }; - this.handleError( - error, - '[ScriptManager] Failed to prefetch script:', - code ? `[${code}]` : '', - script.toObject() - ); + const uniqueId = Script.getScriptUniqueId(scriptId, caller); + if (this.scriptsPromises[uniqueId]) { + return this.scriptsPromises[uniqueId]; } + const loadProcess = async () => { + const script = await this.resolveScript(scriptId, caller, webpackContext); + + try { + this.emit('prefetching', script.toObject()); + await this.nativeScriptManager.prefetchScript(scriptId, script.locator); + } catch (error) { + const { code } = error as Error & { code: string }; + this.handleError( + error, + '[ScriptManager] Failed to prefetch script:', + code ? `[${code}]` : '', + script.toObject() + ); + } + + delete this.scriptsPromises[uniqueId]; + }; + + this.scriptsPromises[uniqueId] = loadProcess(); + this.scriptsPromises[uniqueId].isPrefetch = true; + return this.scriptsPromises[uniqueId]; } /** @@ -422,7 +458,10 @@ export class ScriptManager extends EventEmitter { await this.initCache(); const ids = scriptIds.length ? scriptIds : Object.keys(this.cache); - ids.forEach((scriptId) => delete this.cache[scriptId]); + ids.forEach((scriptId) => { + delete this.cache[scriptId]; + delete this.scriptsPromises[scriptId]; + }); await this.saveCache(); await this.nativeScriptManager.invalidateScripts(scriptIds); diff --git a/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts b/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts index db27816a0..2a5161613 100644 --- a/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts +++ b/packages/repack/src/modules/ScriptManager/__tests__/ScriptManager.test.ts @@ -1,4 +1,6 @@ -import NativeScriptManager from '../NativeScriptManager'; +import NativeScriptManager, { + type NormalizedScriptLocator, +} from '../NativeScriptManager'; import { Script } from '../Script'; import { ScriptManager } from '../ScriptManager'; @@ -736,4 +738,114 @@ describe('ScriptManagerAPI', () => { expect(NativeScriptManager.loadScript).toHaveBeenCalledTimes(3); jest.useRealTimers(); }); + + it('should await loadScript with same scriptId to finish', async () => { + const spy = mockLoadScriptBasedOnFetch(); + + const cache = new FakeCache(); + ScriptManager.shared.setStorage(cache); + + ScriptManager.shared.addResolver(async (scriptId, _caller) => { + return { + url: Script.getRemoteURL(scriptId), + cache: true, + }; + }); + + let loadingScriptIsFinished = false; + + // loadScript should wait first time called loadScript although we are not awaited, because scriptId is same + ScriptManager.shared.loadScript('miniApp').then(() => { + loadingScriptIsFinished = true; + }); + await ScriptManager.shared.loadScript('miniApp'); + + expect(loadingScriptIsFinished).toEqual(true); + spy.mockReset(); + spy.mockRestore(); + }); + + it('should wait loadScript with same scriptId to finished in a complex scenario', async () => { + const spy = mockLoadScriptBasedOnFetch(); + + const cache = new FakeCache(); + ScriptManager.shared.setStorage(cache); + + ScriptManager.shared.addResolver(async (scriptId, _caller) => { + return { + url: Script.getRemoteURL(scriptId), + cache: true, + }; + }); + + let loadingScriptIsFinished = false; + let loadingScript2IsFinished = false; + + // loadScript should wait first time called loadScript although we are not awaited, because scriptId is same + ScriptManager.shared.loadScript('miniApp').then(() => { + loadingScriptIsFinished = true; + }); + + ScriptManager.shared.loadScript('miniApp2').then(() => { + loadingScript2IsFinished = true; + }); + await ScriptManager.shared.loadScript('miniApp'); + expect(loadingScriptIsFinished).toEqual(true); + + loadingScriptIsFinished = false; + ScriptManager.shared.loadScript('miniApp').then(() => { + loadingScriptIsFinished = true; + }); + + ScriptManager.shared.loadScript('miniApp2'); + await ScriptManager.shared.loadScript('miniApp'); + + expect(loadingScriptIsFinished).toEqual(true); + await ScriptManager.shared.loadScript('miniApp2'); + expect(loadingScript2IsFinished).toEqual(true); + spy.mockReset(); + spy.mockRestore(); + }); + + it('should wait loadScript and prefetchScript', async () => { + const spy = mockLoadScriptBasedOnFetch(); + + const cache = new FakeCache(); + ScriptManager.shared.setStorage(cache); + + ScriptManager.shared.addResolver(async (scriptId, _caller) => { + return { + url: Script.getRemoteURL(scriptId), + cache: true, + }; + }); + + let prefetchScriptIsFinished = false; + + // loadScript should wait first time called loadScript although we are not awaited, because scriptId is same + ScriptManager.shared.prefetchScript('miniApp').then(() => { + prefetchScriptIsFinished = true; + }); + await ScriptManager.shared.loadScript('miniApp'); + + expect(prefetchScriptIsFinished).toEqual(true); + spy.mockReset(); + spy.mockRestore(); + }); }); + +function mockLoadScriptBasedOnFetch() { + jest.useFakeTimers({ advanceTimers: true }); + const spy = jest.spyOn(NativeScriptManager, 'loadScript'); + + spy.mockImplementation( + (_scriptId: string, scriptConfig: NormalizedScriptLocator) => + scriptConfig.fetch + ? new Promise((resolve) => { + setTimeout(() => resolve(null), 10); + }) + : Promise.resolve(null) + ); + + return spy; +}