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

Layers mount on map in unpredictable manner #101

Open
IlyaIzr opened this issue Aug 12, 2022 · 4 comments
Open

Layers mount on map in unpredictable manner #101

IlyaIzr opened this issue Aug 12, 2022 · 4 comments

Comments

@IlyaIzr
Copy link
Contributor

IlyaIzr commented Aug 12, 2022

The issue is: some layers sometimes doesn't load on map. See this test stage case I've caught
image
This error happends 'cause we were trying to add source to map that wasn't done loading.
We've fixed that in willMount lifecycle hook for both generic and bivariate layers. But here it was caused by willSourceUpdate hook.

So the natural solution for me was to move fix one step higher, something like that

  private async _updateMap(
    map: ApplicationMap,
    layerData: LayerSource,
    legend: LayerLegend | null,
    isVisible: boolean,
  ) {
    if (layerData == null) return;
// adding this chunk
    if (!map.loaded()) {
      (await waitMapEvent(map, 'load'));
    }
// the wait here can become infinite
    if (this.isGeoJSONLayer(layerData)) {
      this.mountGeoJSONLayer(map, layerData, legend);
    } else {
      this.mountTileLayer(map, layerData, legend);
    }

But that wouldn't work - map won't mount layers. Now the interesting part - let's log our waitMapEvent helper

export const waitMapEvent = <T extends keyof MapEventType>(
  map: ApplicationMap,
  event: T,
) =>
  new Promise<MapEventType[T] & EventData | void>((res, rej) => {
    try {
      console.log('%c⧭ is map loaded already?', 'color: #735656', map._loaded, map.loaded());
      map.on(event, res);
    } catch (error) {
      rej(error);
    }
  });

When mounting fails following console logs can be seen
image

This matters hugely, because map.on('load', callback) will not run, if it was setted after the map was actually loaded.
And if we need to rely on map readyness (and we do) we need to rely on map._loaded property

In other words example above could be working with on line fix

  private async _updateMap(
    map: ApplicationMap,
    layerData: LayerSource,
    legend: LayerLegend | null,
    isVisible: boolean,
  ) {
    if (layerData == null) return;
// adding this chunk
    if (!map._loaded) {
      (await waitMapEvent(map, 'load'));
    }
// the wait here can become infinite
    if (this.isGeoJSONLayer(layerData)) {
      this.mountGeoJSONLayer(map, layerData, legend);
    } else {
      this.mountTileLayer(map, layerData, legend);
    }
@Akiyamka
Copy link
Contributor

Akiyamka commented Aug 12, 2022

Underscore in property name means that it only for internal use of mapbox itself, and we shouldn't relay on this.
From that point of view it's ok that mapbox in first report that it ready for internal staff, then expose readyness to outside (i expect that .ready() have more checks)

@Akiyamka
Copy link
Contributor

Akiyamka commented Aug 12, 2022

This matters hugely, because map.on('load', callback) will not run, if it was setted after the map was actually loaded.

If that was the case, the callback would never have been triggered, app will never load layers that waits until map load, right?
But and the console error says the opposite - that we started something before the map was ready

@IlyaIzr
Copy link
Contributor Author

IlyaIzr commented Aug 14, 2022

(i expect that .ready() have more checks)

I don't know such method for maplibre map, what am I missing?
image

I know that we shouldn't rely on underscore props, that's why I'm describing the problem so thoroughly:)

But and the console error says the opposite - that we started something before the map was ready

Console says we registred\assigned callback for map 'load' event and indeed it never happend. But map.loaded() said that map wasn't ready yet, so we would expect map to fire 'load' event callback.

@Akiyamka
Copy link
Contributor

Akiyamka commented Aug 15, 2022

I don't know such method for maplibre map

Sorry i mean .loaded() of course

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants