From 971b0740857b8758f14d7e4b9808037f3a987339 Mon Sep 17 00:00:00 2001 From: melitele Date: Sun, 25 Jan 2026 12:38:00 -0700 Subject: [PATCH 1/2] pass loading function instead of url to `setRTLTextPlugin` `tilerenderer` doesn't handle network communication directly for tiles, there is no reason to do it for pulling the RTL plugin it also allows to handle RTL pulgin exposing an instantiation method that doesn't rely on registering a callback in global space --- package.json | 1 - src/index.js | 19 ++-- src/source/rtl_text_plugin.js | 55 ++-------- test/unit/source/rtl_text_plugin.test.js | 13 +-- .../source/rtl_text_plugin_loader.test.js | 103 ++++-------------- 5 files changed, 48 insertions(+), 143 deletions(-) diff --git a/package.json b/package.json index 4b38f7575..c4b25fe91 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,6 @@ "@mapwhit/vector-tile": "4.0.0", "@pirxpilot/nanoassert": "~1", "csscolorparser": "^1.0.3", - "dynload": "^1.0.2", "earcut": "^3.0.1", "geojson-vt": "^4.0.2", "grid-index": "^1.1.0", diff --git a/src/index.js b/src/index.js index bd773c777..3844bd56c 100644 --- a/src/index.js +++ b/src/index.js @@ -43,22 +43,27 @@ export default mapwhit; */ /** - * Sets the map's [RTL text plugin](https://www.mapbox.com/mapbox-gl-js/plugins/#mapbox-gl-rtl-text). + * Sets the map's [RTL text plugin](https://github.com/mapwhit/rtl-text). * Necessary for supporting languages like Arabic and Hebrew that are written right-to-left. * * @function setRTLTextPlugin - * @param {string} pluginURL URL pointing to the Mapbox RTL text plugin source. - * @param {boolean} lazy If set to `true`, mapboxgl will defer loading the plugin until rtl text is encountered, + * @param {function} loadPlugin a function that returns a Promise resolving to object + * with RTL text plugin methods `applyArabicShaping`, `processBidirectionalText`, + * and `processStyledBidirectionalText`. + * @param {boolean} lazy If set to `true`, loading the plugin will defer until rtl text is encountered, * rtl text will then be rendered only after the plugin finishes loading. * @example - * setRTLTextPlugin('https://unpkg.com/@mapbox/mapbox-gl-rtl-text@0.3.0/dist/mapbox-gl-rtl-text.js', false); + * ```javascript + * import loadRTLTextPlugin from '@mapwhit/rtl-text'; + * setRTLTextPlugin(loadRTLTextPlugin, true); + * ``` * @see [Add support for right-to-left scripts](https://maplibre.org/maplibre-gl-js/docs/examples/mapbox-gl-rtl-text/) */ -function setRTLTextPlugin(pluginURL, lazy) { - return rtlPluginLoader.setRTLTextPlugin(pluginURL, lazy); +function setRTLTextPlugin(loadPlugin, lazy) { + return rtlPluginLoader.setRTLTextPlugin(loadPlugin, lazy); } /** - * Gets the map's [RTL text plugin](https://www.mapbox.com/mapbox-gl-js/plugins/#mapbox-gl-rtl-text) status. + * Gets the map's [RTL text plugin](https://github.com/mapwhit/rtl-text) status. * The status can be `unavailable` (i.e. not requested or removed), `loading`, `loaded` or `error`. * If the status is `loaded` and the plugin is requested again, an error will be thrown. * diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index 8bef13bd5..4e332a423 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -1,6 +1,4 @@ import { Event, Evented } from '@mapwhit/events'; -import dynload from 'dynload'; -import browser from '../util/browser.js'; /** * The possible option of the plugin's status @@ -49,14 +47,13 @@ export const rtlPlugin = RTLPlugin(); function RTLPluginLoader() { let status = 'unavailable'; - let url; + let load; const self = { getRTLTextPluginStatus, setRTLTextPlugin, lazyLoad, - _clearRTLTextPlugin, - _registerRTLTextPlugin + _clearRTLTextPlugin }; /** This one is exposed to outside */ @@ -66,20 +63,17 @@ function RTLPluginLoader() { // public for testing function _clearRTLTextPlugin() { - url = undefined; status = 'unavailable'; + load = undefined; _setMethods(); } - function setRTLTextPlugin(pluginURL, deferred = false) { - if (url) { + function setRTLTextPlugin(pluginLoad, deferred = false) { + if (load) { // error return Promise.reject(new Error('setRTLTextPlugin cannot be called multiple times.')); } - url = browser.resolveURL(pluginURL); - if (!url) { - return Promise.reject(new Error(`requested url ${pluginURL} is invalid`)); - } + load = pluginLoad; if (status === 'requested') { return _downloadRTLTextPlugin(); } @@ -93,9 +87,13 @@ function RTLPluginLoader() { } async function _downloadRTLTextPlugin() { + if (typeof load !== 'function') { + return Promise.reject(new Error('RTL text plugin load function is not set.')); + } status = 'loading'; try { - await rtlPluginLoader._loadScript({ url }); + _setMethods(await load()); + status = 'loaded'; } catch { status = 'error'; } @@ -126,36 +124,7 @@ function RTLPluginLoader() { rtlPlugin.processStyledBidirectionalText = rtlTextPlugin.processStyledBidirectionalText; } - // This is invoked by the RTL text plugin when the download has finished, and the code has been parsed. - function _registerRTLTextPlugin(rtlTextPlugin) { - if (rtlPlugin.isRTLSupported()) { - throw new Error('RTL text plugin already registered.'); - } - status = 'loaded'; - _setMethods(rtlTextPlugin); - } - return self; } -// public for testing -function _loadScript({ url }) { - const { promise, resolve, reject } = Promise.withResolvers(); - const s = dynload(url); - s.onload = () => resolve(); - s.onerror = () => reject(true); - return promise; -} - -const { getRTLTextPluginStatus, setRTLTextPlugin, lazyLoad, _clearRTLTextPlugin, _registerRTLTextPlugin } = - RTLPluginLoader(); - -globalThis.registerRTLTextPlugin ??= _registerRTLTextPlugin; - -export const rtlPluginLoader = Object.assign(new Evented(), { - getRTLTextPluginStatus, - setRTLTextPlugin, - lazyLoad, - _clearRTLTextPlugin, - _loadScript -}); +export const rtlPluginLoader = Object.assign(new Evented(), RTLPluginLoader()); diff --git a/test/unit/source/rtl_text_plugin.test.js b/test/unit/source/rtl_text_plugin.test.js index 88bef50d9..5d4dacfb4 100644 --- a/test/unit/source/rtl_text_plugin.test.js +++ b/test/unit/source/rtl_text_plugin.test.js @@ -27,13 +27,13 @@ test('RTLPlugin', async t => { t.assert.ok(rtlPlugin.processStyledBidirectionalText == null); }); - await t.test('plugin loaded', t => { + await t.test('plugin loaded', async t => { const rtlTextPlugin = { applyArabicShaping: () => {}, processBidirectionalText: () => {}, processStyledBidirectionalText: () => {} }; - globalThis.registerRTLTextPlugin(rtlTextPlugin); + await rtlPluginLoader.setRTLTextPlugin(() => Promise.resolve(rtlTextPlugin)); t.assert.ok(rtlPlugin.isRTLSupported()); t.assert.ok(rtlPlugin.isRTLSupported(true)); t.assert.equal(rtlPlugin.applyArabicShaping, rtlTextPlugin.applyArabicShaping); @@ -42,11 +42,7 @@ test('RTLPlugin', async t => { }); await t.test('plugin deferred', async t => { - t.mock.method(rtlPluginLoader, '_loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); - await rtlPluginLoader.setRTLTextPlugin('http://example.com/plugin', true); + await rtlPluginLoader.setRTLTextPlugin(() => new Promise(), true); t.assert.ok(!rtlPlugin.isRTLSupported()); t.assert.ok(rtlPlugin.isRTLSupported(true)); }); @@ -58,9 +54,8 @@ test('RTLPlugin', async t => { }); await t.test('plugin download failed', async t => { - t.mock.method(rtlPluginLoader, '_loadScript', () => Promise.reject()); try { - await rtlPluginLoader.setRTLTextPlugin('http://example.com/plugin'); + await rtlPluginLoader.setRTLTextPlugin(() => Promise.reject()); } catch {} t.assert.ok(!rtlPlugin.isRTLSupported()); t.assert.ok(rtlPlugin.isRTLSupported(true)); diff --git a/test/unit/source/rtl_text_plugin_loader.test.js b/test/unit/source/rtl_text_plugin_loader.test.js index d5e876d45..2ac0190d0 100644 --- a/test/unit/source/rtl_text_plugin_loader.test.js +++ b/test/unit/source/rtl_text_plugin_loader.test.js @@ -1,16 +1,8 @@ import test from 'node:test'; -import { rtlPlugin, rtlPluginLoader } from '../../../src/source/rtl_text_plugin.js'; -import browser from '../../../src/util/browser.js'; +import { rtlPluginLoader } from '../../../src/source/rtl_text_plugin.js'; import { sleep } from '../../util/util.js'; -import _window from '../../util/window.js'; test('RTLPluginLoader', async t => { - let globalWindow; - const url = 'http://example.com/plugin'; - t.before(() => { - globalWindow = globalThis.window; - globalThis.window = _window; - }); t.beforeEach(() => { // Reset the singleton instance before each test rtlPluginLoader._clearRTLTextPlugin(); @@ -18,9 +10,6 @@ test('RTLPluginLoader', async t => { t.afterEach(() => { t.mock.reset(); }); - t.after(() => { - globalThis.window = globalWindow; - }); await t.test('should get the RTL text plugin status', t => { const status = rtlPluginLoader.getRTLTextPluginStatus(); @@ -28,57 +17,43 @@ test('RTLPluginLoader', async t => { }); await t.test('should set the RTL text plugin and download it', async t => { - t.mock.method(rtlPluginLoader, '_loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); - const promise = rtlPluginLoader.setRTLTextPlugin(url); - await sleep(0); - await promise; + await rtlPluginLoader.setRTLTextPlugin(() => Promise.resolve({})); t.assert.deepEqual(rtlPluginLoader.getRTLTextPluginStatus(), 'loaded'); - t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 1); }); await t.test('should set the RTL text plugin but defer downloading', async t => { - t.mock.method(rtlPluginLoader, '_loadScript'); - await rtlPluginLoader.setRTLTextPlugin(url, true); - t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 0); + await rtlPluginLoader.setRTLTextPlugin(() => Promise.resolve({}), true); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'deferred'); }); await t.test('should throw if the plugin is already set', async t => { - await rtlPluginLoader.setRTLTextPlugin(url, true); - await t.assert.rejects(rtlPluginLoader.setRTLTextPlugin(url), { - message: 'setRTLTextPlugin cannot be called multiple times.' - }); + await rtlPluginLoader.setRTLTextPlugin(() => Promise.resolve({}), true); + await t.assert.rejects( + rtlPluginLoader.setRTLTextPlugin(() => Promise.resolve({})), + { + message: 'setRTLTextPlugin cannot be called multiple times.' + } + ); }); - await t.test('should throw if the plugin url is not set', async t => { - t.mock.method(browser, 'resolveURL', () => ''); + await t.test('should reject if the plugin load function is not set', async t => { await t.assert.rejects(rtlPluginLoader.setRTLTextPlugin(null), { - message: 'requested url null is invalid' + message: 'RTL text plugin load function is not set.' }); }); - await t.test('should be in error state if download fails', async t => { - t.mock.method(rtlPluginLoader, '_loadScript', () => Promise.reject()); - await rtlPluginLoader.setRTLTextPlugin(url); + await t.test('should be in error state if load fails', async t => { + await rtlPluginLoader.setRTLTextPlugin(() => Promise.reject()); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'error'); }); await t.test('should lazy load the plugin if deferred', async t => { - t.mock.method(rtlPluginLoader, '_loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); - await rtlPluginLoader.setRTLTextPlugin(url, true); - t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 0); + const loadFn = t.mock.fn(() => Promise.resolve({})); + await rtlPluginLoader.setRTLTextPlugin(loadFn, true); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'deferred'); - const promise = rtlPluginLoader.lazyLoad(); - await sleep(0); - await promise; + await rtlPluginLoader.lazyLoad(); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'loaded'); - t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 1); + t.assert.equal(loadFn.mock.callCount(), 1); }); await t.test('should set status to requested if RTL plugin was not set', t => { @@ -87,15 +62,11 @@ test('RTLPluginLoader', async t => { }); await t.test('should immediately download if RTL plugin was already requested, ignoring deferred:true', async t => { - t.mock.method(rtlPluginLoader, '_loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); rtlPluginLoader.lazyLoad(); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'requested'); await sleep(1); // notice even when deferred is true, it should download because already requested - await rtlPluginLoader.setRTLTextPlugin(url, true); + await rtlPluginLoader.setRTLTextPlugin(() => Promise.resolve(), true); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'loaded'); }); @@ -107,44 +78,10 @@ test('RTLPluginLoader', async t => { }); await t.test('should be in error state if lazyLoad fails', async t => { - const resultPromise = rtlPluginLoader.setRTLTextPlugin(url, true); - t.assert.equal(await resultPromise, undefined); + await rtlPluginLoader.setRTLTextPlugin(() => Promise.reject(), true); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'deferred'); // the next one should fail - t.mock.method(rtlPluginLoader, '_loadScript', () => Promise.reject()); await rtlPluginLoader.lazyLoad(); t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'error'); }); - - await t.test('should throw if already parsed', t => { - const rtlTextPlugin = { - applyArabicShaping: () => {}, - processBidirectionalText: () => {}, - processStyledBidirectionalText: () => {} - }; - globalThis.registerRTLTextPlugin(rtlTextPlugin); - t.assert.throws(() => globalThis.registerRTLTextPlugin(rtlTextPlugin), { - message: 'RTL text plugin already registered.' - }); - }); - - await t.test('should not change RTL plugin status if already parsed', t => { - const rtlTextPlugin = { - applyArabicShaping: () => {}, - processBidirectionalText: () => {}, - processStyledBidirectionalText: () => {} - }; - globalThis.registerRTLTextPlugin(rtlTextPlugin); - const rtlTextPlugin2 = { - applyArabicShaping: () => {}, - processBidirectionalText: () => {}, - processStyledBidirectionalText: () => {} - }; - try { - globalThis.registerRTLTextPlugin(rtlTextPlugin2); - } catch {} - t.assert.equal(rtlPlugin.applyArabicShaping, rtlTextPlugin.applyArabicShaping); - t.assert.equal(rtlPlugin.processBidirectionalText, rtlTextPlugin.processBidirectionalText); - t.assert.equal(rtlPlugin.processStyledBidirectionalText, rtlTextPlugin.processStyledBidirectionalText); - }); }); From 3228b7713d52fe33267d3447552fb5b36e1d9243 Mon Sep 17 00:00:00 2001 From: melitele Date: Tue, 27 Jan 2026 19:09:34 -0700 Subject: [PATCH 2/2] instantiate RTL plugin for render tests --- test/integration/lib/suite_implementation.js | 13 +++++-------- test/integration/package.json | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/test/integration/lib/suite_implementation.js b/test/integration/lib/suite_implementation.js index 92bdcccbb..e3a64c8b2 100644 --- a/test/integration/lib/suite_implementation.js +++ b/test/integration/lib/suite_implementation.js @@ -1,5 +1,7 @@ import path from 'node:path'; import { setTimeout } from 'node:timers/promises'; +import rtlText from '@mapwhit/rtl-text'; +import { setRTLTextPlugin } from '../../../src/index.js'; import Map from '../../../src/ui/map.js'; import browser from '../../../src/util/browser.js'; import config from '../../../src/util/config.js'; @@ -8,17 +10,12 @@ import { readPNG } from './png.js'; globalThis.window ??= _window; -async function loadPlugin() { - const { default: rtlText } = await import('@mapwhit/rtl-text'); - return await rtlText(); -} - let pluginloaded; export default async function suiteImplementation(style, options) { - if (options.loadRTLTextPlugin) { - pluginloaded ??= loadPlugin(); - await pluginloaded; + if (options.loadRTLTextPlugin && !pluginloaded) { + pluginloaded = true; + await setRTLTextPlugin(rtlText); } window.devicePixelRatio = options.pixelRatio; diff --git a/test/integration/package.json b/test/integration/package.json index 8cd9fa272..c1c29ca5a 100644 --- a/test/integration/package.json +++ b/test/integration/package.json @@ -1,7 +1,7 @@ { "devDependencies": { "@mapbox/mvt-fixtures": "^3.2.0", - "@mapwhit/rtl-text": "^0.0.2", + "@mapwhit/rtl-text": "^0.1.0", "pixelmatch": "^6.0.0", "pngjs": "^7.0.0" },