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

styledata event can cause "Style is not done loading" #9779

Open
hyperknot opened this issue Jun 11, 2020 · 3 comments
Open

styledata event can cause "Style is not done loading" #9779

hyperknot opened this issue Jun 11, 2020 · 3 comments
Labels
needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else)

Comments

@hyperknot
Copy link

hyperknot commented Jun 11, 2020

mapbox-gl-js version: 1.10.2

browser: various from Sentry: Firefox 77 / Mobile Safari 13 / Edge 16 / Firefox Mobile 67 / Chrome 59

Steps to Trigger Behavior

  1. Initialize the map with an empty style.
  2. After the load event, call setStyle() with an actual style (url or object)
  3. add a source in a styledata callback
map.once('styledata', _ => {
    map.addSource(...) // => "Style is not done loading"
  })

The setStyle could even be an empty style with {diff: false}, meaning nothing is loaded at all, it'd still trigger the bug!

Link to Demonstration

I cannot reproduce it locally, but it's showing up in Sentry issues quite a lot.

Expected Behavior

It should be possible to add a source in a styledata event callback, as the style should be loaded at that time.

By the documentation, styledata event should only trigger once the style has loaded. styledataloading is the event which should trigger once the style starts loading, but hasn't loaded yet.

Actual Behavior

styledata is not working correctly, as it can be triggered before the style has finished loading. Cannot reproduce it locally, but it is happening to many users in Sentry.

The bigger problem here is that I'd like to know what is a reliable way to detect style load events. I'd like to dynamically switch styles and add a source after each switching. Basically https://docs.mapbox.com/mapbox-gl-js/example/setstyle/ + adding a source.

StackOverflow is full of weird workarounds for this or very similar issues.

  • using setTimeout + isStyleLoaded in a loop
  • recommending style.load event (officially not supported)
  • recommending idle (which would make the whole app super slow, as it wouldn't just wait for the style to load but for all tiles to finish loading)

Even issues here have many recommendation for setTimeout + isStyleLoaded or other weird workarounds.

It just feels very much not on the otherwise super high standard of this library that such basic functionality is not working correctly. Dynamic style switching + adding custom sources must be like one of the most common use cases of Mapbox GL JS, I'm really surprised it has so many issues around it. Personally I find the cleanest to use style.load but then it's not official for some reason.

@hyperknot
Copy link
Author

I've just checked the source code, and basically styledata and style.load should behave exactly the same as they are at the very same part of the code:

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

I'll check for a day with style.load and report if the errors persist or not.

@hyperknot
Copy link
Author

hyperknot commented Jun 11, 2020

I can confirm that changing to "style.load" totally fixed the issue.

What might be the reason is that there is only one occurance of this.fire(new Event('style.load')) in the whole codebase, whereas there are 5 occurances of this.fire(new Event('data', {dataType: 'style'})) .

I believe what might be happening is that some of those 5 styledata events can get fired before the final styledata / style.load point, and that can cause this bug.

Why are there 5 occurances of styledata in the codebase if only one is matching what's written in the documentation?

@mourner mourner added the needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else) label Jul 6, 2020
@stevage
Copy link
Contributor

stevage commented Oct 22, 2020

It just feels very much not on the otherwise super high standard of this library that such basic functionality is not working correctly.

Yes: very, very, very much this. I'm running into this issue yet again, and it's a massive pain point.

See also #8691.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else)
Projects
None yet
Development

No branches or pull requests

3 participants