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

loaded() and on('load') event do not work as expected #6707

Open
gmaclennan opened this issue May 22, 2018 · 20 comments
Open

loaded() and on('load') event do not work as expected #6707

gmaclennan opened this issue May 22, 2018 · 20 comments
Labels

Comments

@gmaclennan
Copy link
Contributor

mapbox-gl-js version: 0.45.0

Question

There are certain methods that you cannot call while the map is still loading, such as map.addLayer(). You can use the map.on('load') event to know when it is safe to call these methods, however there is no corresponding method like map.ready() for knowing that it is safe to call map.addLayer(). map.loaded() can also return false during other method calls, such as map.setLayoutProperty(), and you can safely call map.addLayer() when map.loaded() returns false. Also if map.loaded() returns false then map.on('load') only works the first time, so you need to track the initial load state yourself.

Every time I have tried to make a complex map with mapbox-gl I have hit some kind of frustrating bug that was related to loading state and these events. They have changed gradually over versions but they still don't work as expected.

It would be great if mapbox-gl could just track its ready state internally, so that calling map.addLayer() just worked as expected straight away. Either that or we need methods that correspond to load events. I want to do something like this:

if (map.loaded()) {
  map.addLayer(...)
} else {
  map.on('load', () => map.addLayer(...))
}

But this will only work the first time (map.on('load') only fires once), but sometimes map.loaded() will return false if the map is moving.

Links to related documentation

@anandthakker
Copy link
Contributor

Thanks for describing this pain point @gmaclennan.

You're right: map.loaded() answers both whether the style has been initially loaded and whether there have been source/layer changes that haven't yet been processed. In general, it's safe to do things like addLayer() any time after the former is true, regardless of the latter.

I think adding a map.hasStyle() method that answers only the first question (and is, I think, what map.ready() in the OP is after), makes sense.

@gmaclennan
Copy link
Contributor Author

Thanks for the quick answer @anandthakker I think that would help this, along with perhaps update to the docs that cross-reference events with corresponding methods.

I've realized that I am using map.on('style.load') in my code to be able to do map.addLayer() as soon as possible, since map.on('load') does not fire until after all the tiles have loaded too (I think). map.on('styledata') looks like it might work for this, but it does not seem to only fire for the initial loading of the style, it also seems to fire for style changes. Is there a supported alternative to style.load event?

Thanks for looking into this. I've been meaning to document the pain points I keep hitting every time for a while.

@anandthakker
Copy link
Contributor

anandthakker commented May 22, 2018

I think style.load is equivalent to the first styledata:

this.fire(new Event('data', {dataType: 'style'}));
this.fire(new Event('style.load'));

(Confusingly, this.fire(new Event('data', {dataType: 'style'})) results in a styledata event)

styledata would also be fired subsequently, after other changes to the style have been applied: e.g., loading the 'sprite' sheet (a separate resource), adding/removing sources, adding and removing images

@gmaclennan
Copy link
Contributor Author

Thanks. I am continuing to experiment with this, and hit another strange behavior: I was trying to get layers to fade in after the tiles have loaded for certain map views. I am doing this:

if (map.areTilesLoaded()) doSomething()
else map.on('tiledata', doSomething)

However tiledata does not always fire after a map.areTilesLoaded(). Again I'm hitting the problem of if the tiles aren't loaded how do I know when they are? Should I try to open another issue on this? Is the expected behavior that these events should match the methods or am I misunderstanding something?

@gmaclennan
Copy link
Contributor Author

I have been using some patterns for common map lifecycle events that I have found useful for projects I have written, these are the functions I'm currently using, they may be useful for others. These were not obvious for me from the docs, and are based on reading through the source code to try to understand the lifecycles, I'm not 100% sure they are correct.

/**
 * Call a function when the map's style has loaded. You can safely add and
 * modify map layers within this function
 *
 * @param {mapboxgl.Map} map
 * @param {function} fn
 */
function onMapStyleLoaded (map, fn) {
  if (map.isStyleLoaded()) return process.nextTick(fn)
  map.once('styledata', () => onMapStyleLoaded(map, fn))
}

/**
 * Call a function when the map's tiles have loaded, i.e. most network activity
 * is complete (although sprite and font loading may still be occurring)
 *
 * @param {mapboxgl.Map} map
 * @param {function} fn
 */
function onMapTilesLoaded (map, fn) {
  if (map.areTilesLoaded()) return process.nextTick(fn)
  map.once('sourcedata', () => onMapTilesLoaded(map, fn))
}

/**
 * Call a function when the map has finished rendering and transitions are
 * complete
 *
 * @param {mapboxgl.Map} map
 * @param {function} fn
 */
function onMapRenderComplete (map, fn) {
  if (map.loaded()) return process.nextTick(fn)
  map.once('render', () => onMapRenderComplete(map, fn))
}

@stevage
Copy link
Contributor

stevage commented Jun 21, 2018

See also the long discussion in #6076. There definitely seems to be a mismatch between, on the one hand, the events, status functions and documentation, and on the other, what users expect and need in order to predictably manipulate the map state without triggering errors.

@stevage
Copy link
Contributor

stevage commented Jun 26, 2018

Just want to add a very concrete demonstration that load does not do what the docs say it does. Specifically:

Fired immediately after all necessary resources have been downloaded and the first visually complete rendering of the map has occurred.

See this demo: https://bl.ocks.org/stevage/raw/96066b4b8914ee40309260340db509eb/

Open the console. It does a count using queryRenderedFeatures() in response to load and render events:

screenshot 2018-06-26 11 45 35

The load count ought to be 3. Instead it varies, sometimes 0, sometimes 1.

As far as I can tell, there is no event you can listen to that is fired after the first render is actually complete and it's safe to call queryRenderedFeatures().

@jwschram
Copy link

jwschram commented Jul 2, 2018

I have also experienced that load handlers do not trigger on maps that are already loaded. For me this has led to an edge case, causing a race condition with load.

I am adding GeoJSON data to a map from a different component. Since it's possible that the data is ready before the map can receive it, or even exists, I'm polling the map object to see if it exists, and if loaded() === true if it is, the data is loaded directly. If the map object exists but map.loaded() === false than a map.once('load;, ...) handler is registered.

The race condition blows up when isLoaded returns false, but the map is actually loaded because the data is not set directly, and the handler isn't called.

One of the solutions mentioned above would help or adopting the jQuery idea that load handlers registered after the map is loaded are called immediately would also help.

@stevage
Copy link
Contributor

stevage commented Aug 16, 2018

@gmaclennan Thanks for your suggestions above. I found I had to modify onMapTilesLoaded a bit more to get it to work for me:

    map.onMapTilesLoaded = function(sourceId, cb) {
        process.nextTick(() => {
            if (map.areTilesLoaded(sourceId)) {
                cb();
            } else {
                map.once('sourcedata', () => map.onMapTilesLoaded(sourceId, cb));
            }
        });
    };

So complicated! My use case here is a user selecting a boundary, then switching to a different dataset with equivalent boundaries. I need to wait for the new dataset to load, then identify the comparable boundary to select.

This method above did eventually work...but yeah, it's very sub-optimal having to repeatedly poll areTilesLoaded() and also call process.nextTick(). (If you don't do the latter map.areTilesLoaded() can return true, yet fail to find the source features you want...and a tick later it will return false.)

@stevage
Copy link
Contributor

stevage commented Oct 17, 2018

I ran into another issue today. map.on("load") never fires, nor is map.loaded() true for a map with no style.

@vanhumbeecka
Copy link

This problem is also causing issues with the mapbox-gl-directions plugin, since it uses this logic internally:

if (map.loaded()) {
  this.mapState()
} else {
  map.on('load', () => map.mapState())
}

...resulting in mapState() that is never run in my case.
(.loaded() is false and on('load') has already run in some situations)

Using mapbox-gl 0.54.0

@stevage
Copy link
Contributor

stevage commented May 7, 2019

Today I found that @gmaclennan's first version (onMapStyleLoaded) sometimes doesn't work. It fires twice, with isStyleLoaded() returning false, but then the styledata doesn't fire the next time.

I'm surprised this issue doesn't get higher priority from the devs. I am still finding this a major pain point in different projects. "Add data and layers at runtime" is a core use case for the library, and it is difficult to do reliably.

@gmaclennan
Copy link
Contributor Author

Yes we've continued to hit bugs with not being able to do this reliably. I would love to see a solution, unfortunately this is all too deep within the code of mapbox-gl-js for me to even start a fix myself beyond workarounds.

@bryanrideshark
Copy link

bryanrideshark commented May 23, 2019

What about just polling map.loaded()?
I'm trying the following:

        const loop = () => {
          if (mapboxMap.loaded()) {
            initialize();
          } else {
            requestAnimationFrame(loop);
          }
        };
        loop();

@vanhumbeecka
Copy link

For anyone interested in a workaround: Take a look at mapbox/mapbox-gl-directions#111

Here, the fix described is using the internal _loaded field, which is not meant to be visible by the public API. Nevertheless, the behaviour of this field is the one that is needed in this case.
(the fix dates back from early 2017, but still works 😄 )

@CodeAmend
Copy link

I really do not mean to bump this, but I am looking for a list of mapbox events. I cannot find them in the docs. So far, what I am doing is using something like ack "fire\(new Event" node_modules/mapbox-gl/ to find Events, but I need some docs. Certainly, this thread has helped a bit. Thank you.

@ryanhamley
Copy link
Contributor

@CodeAmend All the publicly available events are listed in our docs. You can view all the map events starting here for example.

@rajatmasih-web4realty
Copy link

mapbox-gl-js version: 0.45.0

Question

There are certain methods that you cannot call while the map is still loading, such as map.addLayer(). You can use the map.on('load') event to know when it is safe to call these methods, however there is no corresponding method like map.ready() for knowing that it is safe to call map.addLayer(). map.loaded() can also return false during other method calls, such as map.setLayoutProperty(), and you can safely call map.addLayer() when map.loaded() returns false. Also if map.loaded() returns false then map.on('load') only works the first time, so you need to track the initial load state yourself.

Every time I have tried to make a complex map with mapbox-gl I have hit some kind of frustrating bug that was related to loading state and these events. They have changed gradually over versions but they still don't work as expected.

It would be great if mapbox-gl could just track its ready state internally, so that calling map.addLayer() just worked as expected straight away. Either that or we need methods that correspond to load events. I want to do something like this:

if (map.loaded()) {
  map.addLayer(...)
} else {
  map.on('load', () => map.addLayer(...))
}

But this will only work the first time (map.on('load') only fires once), but sometimes map.loaded() will return false if the map is moving.

Links to related documentation

Thanks man

@stevage
Copy link
Contributor

stevage commented Feb 14, 2024

I have just run into this problem again, in Mapbox GL JS 3.0.1, with mapbox-gl-draw.

An intermittent bug, where my code worked fine for a while...until it suddenly doesn't.

You can see that mapbox-gl-draw uses exactly the pattern described above:
https://github.com/mapbox/mapbox-gl-draw/blob/main/src/setup.js#L75

if (map.loaded()) {
        setup.connect();
      } else {
        map.on('load', setup.connect);
        mapLoadedInterval = setInterval(() => { if (map.loaded()) setup.connect(); }, 16);
      }

In my app, I'm adding the Draw object to the map after load has fired once. So you'd think that map.loaded() would return true. Sometimes yes, but sometimes no.

Super frustrating.

@stevage
Copy link
Contributor

stevage commented Feb 14, 2024

Huh, I have just realised that an easy workaround in my case is to simply do add this:

map.fire("load");

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

No branches or pull requests

9 participants