maplibre / maplibre-gl-js

MapLibre GL JS - Interactive vector tile maps in the browser
https://maplibre.org/maplibre-gl-js/docs/
Other
5.98k stars 659 forks source link

RTL plugin with `await` triggers console error #4252

Open wipfli opened 3 weeks ago

wipfli commented 3 weeks ago

maplibre-gl-js version:

browser: Chrome on linux

Steps to Trigger Behavior

  1. Create an RTL plugin which has an await statement before the call to self.registerRTLTextPlugin()
  2. Load the plugin and create a map
  3. Look at the browser console, find the error message Error: RTL Text Plugin failed to import scripts from

Link to Demonstration

https://github.com/wipfli/maplibre-rtl-regression

MapLibre GL JS v3.6.1

MapLibre GL JS v4.3.2

Expected Behavior

RTL plugin with await does not trigger a console error.

Actual Behavior

It triggers an error.

Note

Both in v3.6.1 and v4.3.2, MapLibre GL JS displays RTL works as defined in the applyArabicShaping function once the sleep time is over. To see this behavior, open one of the examples with-await links, wait for 2 seconds, then zoom in. The country labels should be replaced by RTL works now.

HarelM commented 3 weeks ago

I'm not entirely sure why there isn't an error in 3.6.1, probably a bug in the code. The current code (and the also in version 3.6.1) assumes that the when importing the rtl script it will populate the 3 methods that are needed for shaping right after the call to import script, i.e. synchronisously. When this happens synchronously, the code in the worker returns a response to the main thread updating about the status. It might be that we need to change that, and only update the state of the main thread after the call to registerRTLTextPlugin is executed to allow an async type of RTL plugin method. CC: @zhangyiatmicrosoft Related PR and some insights on how this all works:

The relevant code is here: https://github.com/maplibre/maplibre-gl-js/blob/8c4cef08ec2899da8ea9b08445b3a7ccd06d9bb5/src/source/worker.ts#L85

https://github.com/maplibre/maplibre-gl-js/blob/8c4cef08ec2899da8ea9b08445b3a7ccd06d9bb5/src/source/worker.ts#L193