diff --git a/CHANGELOG.md b/CHANGELOG.md index a6ff5ab7f0..a05e98c692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### 🐞 Bug fixes - Fix text not being hidden behind the globe when overlap mode was set to `always` ([#4802](https://github.com/maplibre/maplibre-gl-js/issues/4802)) - Fix a single white frame being displayed when the map internally transitions from mercator to globe projection ([#4816](https://github.com/maplibre/maplibre-gl-js/issues/4816)) +- Fix loading of RTL plugin version 0.3.0 ([#4860](https://github.com/maplibre/maplibre-gl-js/pull/4860)) - _...Add new stuff here..._ ## 5.0.0-pre.2 diff --git a/src/index.ts b/src/index.ts index 56cf375388..a27ba96172 100644 --- a/src/index.ts +++ b/src/index.ts @@ -60,7 +60,7 @@ export type * from '@maplibre/maplibre-gl-style-spec'; * rtl text will then be rendered only after the plugin finishes loading. * @example * ```ts - * setRTLTextPlugin('https://unpkg.com/@mapbox/mapbox-gl-rtl-text@0.2.3/mapbox-gl-rtl-text.js', false); + * setRTLTextPlugin('https://unpkg.com/@mapbox/mapbox-gl-rtl-text@0.3.0/dist/mapbox-gl-rtl-text.js', false); * ``` * @see [Add support for right-to-left scripts](https://maplibre.org/maplibre-gl-js/docs/examples/mapbox-gl-rtl-text/) */ diff --git a/src/source/rtl_text_plugin_worker.test.ts b/src/source/rtl_text_plugin_worker.test.ts new file mode 100644 index 0000000000..39993aed4e --- /dev/null +++ b/src/source/rtl_text_plugin_worker.test.ts @@ -0,0 +1,118 @@ +import {PluginState} from './rtl_text_plugin_status'; +import {rtlWorkerPlugin} from './rtl_text_plugin_worker'; + +describe('RTLWorkerPlugin', () => { + beforeEach(() => { + // This is a static class, so we need to reset the properties before each test + rtlWorkerPlugin.processStyledBidirectionalText = null; + rtlWorkerPlugin.processBidirectionalText = null; + rtlWorkerPlugin.applyArabicShaping = null; + }); + + test('should throw if already parsed', () => { + const rtlTextPlugin = { + applyArabicShaping: jest.fn(), + processBidirectionalText: jest.fn(), + processStyledBidirectionalText: jest.fn(), + }; + + rtlWorkerPlugin.setMethods(rtlTextPlugin); + expect(() => { + rtlWorkerPlugin.setMethods(rtlTextPlugin); + }).toThrow('RTL text plugin already registered.'); + }); + + test('should move RTL plugin from unavailable to deferred', async () => { + rtlWorkerPlugin.pluginURL = ''; + rtlWorkerPlugin.pluginStatus = 'unavailable'; + + const mockMessage: PluginState = { + pluginURL: 'https://somehost/somescript', + pluginStatus: 'deferred' + }; + + await rtlWorkerPlugin.syncState(mockMessage, jest.fn()); + + expect(rtlWorkerPlugin.getRTLTextPluginStatus()).toBe('deferred'); + }); + + test('should not change RTL plugin status if already parsed', async () => { + const originalUrl = 'https://somehost/somescript1'; + rtlWorkerPlugin.pluginURL = originalUrl; + rtlWorkerPlugin.pluginStatus = 'loaded'; + rtlWorkerPlugin.setMethods({ + applyArabicShaping: jest.fn(), + processBidirectionalText: jest.fn(), + processStyledBidirectionalText: jest.fn(), + }); + const mockMessage: PluginState = { + pluginURL: 'https://somehost/somescript2', + pluginStatus: 'loading' + }; + + const workerResult: PluginState = await await rtlWorkerPlugin.syncState(mockMessage, jest.fn()); + + expect(rtlWorkerPlugin.getRTLTextPluginStatus()).toBe('loaded'); + expect(rtlWorkerPlugin.pluginURL).toBe(originalUrl); + + expect(workerResult.pluginStatus).toBe('loaded'); + expect(workerResult.pluginURL).toBe(originalUrl); + }); + + test('should do a full cycle of rtl loading synchronously', async () => { + const originalUrl = 'https://somehost/somescript1'; + const loadScriptsMock = jest.fn().mockImplementation((_) => { + rtlWorkerPlugin.setMethods({ + applyArabicShaping: jest.fn(), + processBidirectionalText: jest.fn(), + processStyledBidirectionalText: jest.fn(), + }); + }); + + const workerResult: PluginState = await rtlWorkerPlugin.syncState({ + pluginURL: originalUrl, + pluginStatus: 'loading' + }, loadScriptsMock); + + expect(rtlWorkerPlugin.getRTLTextPluginStatus()).toBe('loaded'); + expect(rtlWorkerPlugin.pluginURL).toBe(originalUrl); + expect(workerResult.pluginStatus).toBe('loaded'); + expect(workerResult.pluginURL).toBe(originalUrl); + }); + + test('should do a full cycle of rtl loading asynchronously', async () => { + const originalUrl = 'https://somehost/somescript1'; + const loadScriptsMock = jest.fn().mockImplementation((_) => { + setTimeout(() => { + rtlWorkerPlugin.setMethods({ + applyArabicShaping: jest.fn(), + processBidirectionalText: jest.fn(), + processStyledBidirectionalText: jest.fn(), + }); + }, 10); + }); + + const workerResult: PluginState = await rtlWorkerPlugin.syncState({ + pluginURL: originalUrl, + pluginStatus: 'loading' + }, loadScriptsMock); + + expect(rtlWorkerPlugin.getRTLTextPluginStatus()).toBe('loaded'); + expect(rtlWorkerPlugin.pluginURL).toBe(originalUrl); + expect(workerResult.pluginStatus).toBe('loaded'); + expect(workerResult.pluginURL).toBe(originalUrl); + }); + + test('should fail loading on timeout', async () => { + const originalUrl = 'https://somehost/somescript1'; + const loadScriptsMock = jest.fn().mockImplementation(() => {}); + + (rtlWorkerPlugin as any).TIMEOUT = 1; + + await expect(rtlWorkerPlugin.syncState({ + pluginURL: originalUrl, + pluginStatus: 'loading' + }, loadScriptsMock) + ).rejects.toThrow('RTL Text Plugin failed to import scripts from https://somehost/somescript1'); + }); +}); diff --git a/src/source/rtl_text_plugin_worker.ts b/src/source/rtl_text_plugin_worker.ts index 88f8d2187e..b01c1efffb 100644 --- a/src/source/rtl_text_plugin_worker.ts +++ b/src/source/rtl_text_plugin_worker.ts @@ -7,42 +7,81 @@ export interface RTLTextPlugin { } class RTLWorkerPlugin implements RTLTextPlugin { + readonly TIMEOUT = 5000; + applyArabicShaping: (a: string) => string = null; processBidirectionalText: ((b: string, a: Array) => Array) = null; processStyledBidirectionalText: ((c: string, b: Array, a: Array) => Array<[string, Array]>) = null; pluginStatus: RTLPluginStatus = 'unavailable'; pluginURL: string = null; + loadScriptResolve: () => void = () => {}; - setState(state: PluginState) { + private setState(state: PluginState) { this.pluginStatus = state.pluginStatus; this.pluginURL = state.pluginURL; } - getState(): PluginState { + private getState(): PluginState { return { pluginStatus: this.pluginStatus, pluginURL: this.pluginURL }; } - setMethods(rtlTextPlugin: RTLTextPlugin) { + public setMethods(rtlTextPlugin: RTLTextPlugin) { + if (rtlWorkerPlugin.isParsed()) { + throw new Error('RTL text plugin already registered.'); + } this.applyArabicShaping = rtlTextPlugin.applyArabicShaping; this.processBidirectionalText = rtlTextPlugin.processBidirectionalText; this.processStyledBidirectionalText = rtlTextPlugin.processStyledBidirectionalText; + this.loadScriptResolve(); } - isParsed(): boolean { + public isParsed(): boolean { return this.applyArabicShaping != null && this.processBidirectionalText != null && this.processStyledBidirectionalText != null; } - getPluginURL(): string { - return this.pluginURL; + public getRTLTextPluginStatus() { + return this.pluginStatus; } - getRTLTextPluginStatus() { - return this.pluginStatus; + public async syncState(incomingState: PluginState, importScripts: (url: string) => void): Promise { + // Parsed plugin cannot be changed, so just return its current state. + if (this.isParsed()) { + return this.getState(); + } + + if (incomingState.pluginStatus !== 'loading') { + // simply sync and done + this.setState(incomingState); + return incomingState; + } + const urlToLoad = incomingState.pluginURL; + const loadScriptPromise = new Promise((resolve) => { + this.loadScriptResolve = resolve; + }); + importScripts(urlToLoad); + const dontWaitForeverTimeoutPromise = new Promise((resolve) => setTimeout(() => resolve(), this.TIMEOUT)); + await Promise.race([loadScriptPromise, dontWaitForeverTimeoutPromise]); + const complete = this.isParsed(); + if (complete) { + const loadedState: PluginState = { + pluginStatus: 'loaded', + pluginURL: urlToLoad + }; + this.setState(loadedState); + return loadedState; + } + + // error case + this.setState({ + pluginStatus: 'error', + pluginURL: '' + }); + throw new Error(`RTL Text Plugin failed to import scripts from ${urlToLoad}`); } } diff --git a/src/source/worker.test.ts b/src/source/worker.test.ts index 7e850b72e5..a973ef797d 100644 --- a/src/source/worker.test.ts +++ b/src/source/worker.test.ts @@ -6,7 +6,6 @@ import {CanonicalTileID, OverscaledTileID} from './tile_id'; import {WorkerSource, WorkerTileParameters, WorkerTileResult} from './worker_source'; import {rtlWorkerPlugin} from './rtl_text_plugin_worker'; import {ActorTarget, IActor} from '../util/actor'; -import {PluginState} from './rtl_text_plugin_status'; import {MessageType} from '../util/actor_messages'; class WorkerSourceMock implements WorkerSource { @@ -37,108 +36,22 @@ describe('Worker RTLTextPlugin', () => { } as any; worker = new Worker(_self); global.fetch = null; - rtlWorkerPlugin.setMethods({ - applyArabicShaping: null, - processBidirectionalText: null, - processStyledBidirectionalText: null - }); - jest.spyOn(rtlWorkerPlugin, 'isParsed').mockImplementation(() => { - return false; - }); - }); - - test('should not throw and set values in plugin', () => { - const rtlTextPlugin = { - applyArabicShaping: 'test', - processBidirectionalText: 'test', - processStyledBidirectionalText: 'test', - }; - - _self.registerRTLTextPlugin(rtlTextPlugin); - expect(rtlWorkerPlugin.applyArabicShaping).toBe('test'); - expect(rtlWorkerPlugin.processBidirectionalText).toBe('test'); - expect(rtlWorkerPlugin.processStyledBidirectionalText).toBe('test'); - }); - - test('should throw if already parsed', () => { - jest.spyOn(rtlWorkerPlugin, 'isParsed').mockImplementation(() => { - return true; - }); - - const rtlTextPlugin = { - applyArabicShaping: jest.fn(), - processBidirectionalText: jest.fn(), - processStyledBidirectionalText: jest.fn(), - }; - - expect(() => { - _self.registerRTLTextPlugin(rtlTextPlugin); - }).toThrow('RTL text plugin already registered.'); }); - test('should move RTL plugin from unavailable to deferred', async () => { - rtlWorkerPlugin.setState({ - pluginURL: '', - pluginStatus: 'unavailable' - } - ); - const mockMessage: PluginState = { - pluginURL: 'https://somehost/somescript', - pluginStatus: 'deferred' - }; - - await worker.actor.messageHandlers[MessageType.syncRTLPluginState]('', mockMessage); - expect(rtlWorkerPlugin.getRTLTextPluginStatus()).toBe('deferred'); - }); + test('should call setMethods in plugin', () => { + const spy = jest.spyOn(rtlWorkerPlugin, 'setMethods').mockImplementation(() => {}); - test('should download RTL plugin when "loading" message is received', async () => { - rtlWorkerPlugin.setState({ - pluginURL: '', - pluginStatus: 'deferred' - }); + _self.registerRTLTextPlugin({} as any); - const mockURL = 'https://somehost/somescript'; - const mockMessage: PluginState = { - pluginURL: mockURL, - pluginStatus: 'loading' - }; - - const importSpy = jest.spyOn(worker.self, 'importScripts').mockImplementation(() => { - // after importing isParse() to return true - jest.spyOn(rtlWorkerPlugin, 'isParsed').mockImplementation(() => { - return true; - }); - }); - - const syncResult: PluginState = await worker.actor.messageHandlers[MessageType.syncRTLPluginState]('', mockMessage) as any; - expect(rtlWorkerPlugin.getRTLTextPluginStatus()).toBe('loaded'); - expect(importSpy).toHaveBeenCalledWith(mockURL); - - expect(syncResult.pluginURL).toBe(mockURL); - expect(syncResult.pluginStatus).toBe('loaded'); + expect(spy).toHaveBeenCalled(); }); - test('should not change RTL plugin status if already parsed', async () => { - const originalUrl = 'https://somehost/somescript1'; - rtlWorkerPlugin.setState({ - pluginURL: originalUrl, - pluginStatus: 'loaded' - }); - - jest.spyOn(rtlWorkerPlugin, 'isParsed').mockImplementation(() => { - return true; - }); - const mockMessage: PluginState = { - pluginURL: 'https://somehost/somescript2', - pluginStatus: 'loading' - }; + test('should call syncState when rtl message is received', async () => { + const syncStateSpy = jest.spyOn(rtlWorkerPlugin, 'syncState').mockImplementation((_, __) => Promise.resolve({} as any)); - const workerResult: PluginState = await worker.actor.messageHandlers[MessageType.syncRTLPluginState]('', mockMessage) as any; - expect(rtlWorkerPlugin.getRTLTextPluginStatus()).toBe('loaded'); - expect(rtlWorkerPlugin.getPluginURL()).toBe(originalUrl); + await worker.actor.messageHandlers[MessageType.syncRTLPluginState]('', {} as any) as any; - expect(workerResult.pluginStatus).toBe('loaded'); - expect(workerResult.pluginURL).toBe(originalUrl); + expect(syncStateSpy).toHaveBeenCalled(); }); }); diff --git a/src/source/worker.ts b/src/source/worker.ts index 5aa69e8722..f0bb811442 100644 --- a/src/source/worker.ts +++ b/src/source/worker.ts @@ -83,9 +83,7 @@ export default class Worker { // This is invoked by the RTL text plugin when the download via the `importScripts` call has finished, and the code has been parsed. this.self.registerRTLTextPlugin = (rtlTextPlugin: RTLTextPlugin) => { - if (rtlWorkerPlugin.isParsed()) { - throw new Error('RTL text plugin already registered.'); - } + rtlWorkerPlugin.setMethods(rtlTextPlugin); }; @@ -191,35 +189,8 @@ export default class Worker { } private async _syncRTLPluginState(mapId: string, incomingState: PluginState): Promise { - - // Parsed plugin cannot be changed, so just return its current state. - if (rtlWorkerPlugin.isParsed()) { - return rtlWorkerPlugin.getState(); - } - - if (incomingState.pluginStatus !== 'loading') { - // simply sync and done - rtlWorkerPlugin.setState(incomingState); - return incomingState; - } - const urlToLoad = incomingState.pluginURL; - this.self.importScripts(urlToLoad); - const complete = rtlWorkerPlugin.isParsed(); - if (complete) { - const loadedState: PluginState = { - pluginStatus: 'loaded', - pluginURL: urlToLoad - }; - rtlWorkerPlugin.setState(loadedState); - return loadedState; - } - - // error case - rtlWorkerPlugin.setState({ - pluginStatus: 'error', - pluginURL: '' - }); - throw new Error(`RTL Text Plugin failed to import scripts from ${urlToLoad}`); + const state = await rtlWorkerPlugin.syncState(incomingState, this.self.importScripts); + return state; } private _getAvailableImages(mapId: string) { diff --git a/test/examples/display-and-style-rich-text-labels.html b/test/examples/display-and-style-rich-text-labels.html index fefbf86511..f97653447d 100644 --- a/test/examples/display-and-style-rich-text-labels.html +++ b/test/examples/display-and-style-rich-text-labels.html @@ -16,7 +16,7 @@