Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTL plugin with await triggers console error #4252

Open
wipfli opened this issue Jun 10, 2024 · 1 comment
Open

RTL plugin with await triggers console error #4252

wipfli opened this issue Jun 10, 2024 · 1 comment
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed

Comments

@wipfli
Copy link
Contributor

wipfli commented Jun 10, 2024

maplibre-gl-js version:

  • error present in 4.3.2
  • error not present in 3.6.1

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
Copy link
Collaborator

HarelM commented Jun 12, 2024

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:

this.self.registerRTLTextPlugin = (rtlTextPlugin: RTLTextPlugin) => {

private async _syncRTLPluginState(mapId: string, incomingState: PluginState): Promise<PluginState> {

@HarelM HarelM added bug Something isn't working PR is more than welcomed Extra attention is needed labels Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants