setStyle(URL) is a race condition - subsequent addLayer() and addSource() possibly lost

[stevage]

mapbox-gl-js version: 1.13.0

Steps to Trigger Behavior

// first initialise map with say 5 layers
map.setStyle('mystyle.json'); // style containing 10 layers)
console.log(map.loaded(), map.isStyleLoaded(), map.getStyle().layers.length);
map.addLayer({ type: 'background', id: 'background' });

setTimeout(()=> console.log(map.getLayer('background')), 5000);

Expected Behavior

false, false, 10 (maybe - unclear what isStyleLoaded should do exactly)

Then print out the background layer we added

Actual Behavior

true, true, 5
undefined

Super frustrating, like #8691, #9779, #2268.

After calling setStyle() (with a JSON) it's difficult/impossible to know when the new style has actually taken effect. If you immediately add new layers, there is no error, but they apparently happen before the actual style change takes place, so they get removed.

[stevage]

Writing this issue up did help me find a workaround that solves the issue:

const styleJSON = (await axios.get('mystyle.json')).data
map.setStyle(styleJSON);

This works fine - by fetching the style ourselves, once we call setStyle() the style is immediately updated, which means our additional layers are not wiped.

[stevage]

I'm actually not really sure what Mapbox GL JS should do here, between calling setStyle() with a new URL and the time that that style has actually loaded. Any of these seem plausible:

• put the map into the "style hasn't finished loading" state immediately
• output warnings whenever any style manipulation happens before the new style has loaded, that those changes will be lost
• queue up style manipulations so they apply after the style has loaded. (Probably not realistic, because how would you handle the "before" parameter of addLayer() - referencing the current style or the future style?)
• take a callback that fires when it's ok to manipulate the style

[andycalder]

Hi @stevage, I think you've raised a really important issue here. map.setStyle(URL) is asynchronous. This being JavaScript, I would expect such a method to take a callback or return a promise so you know when the internal style object has been updated. Ideally the API would let you do something like this:

map.setStyle('mystyle.json')
    .then(map => {
        map.addLayer({...})
    });

or

await map.setStyle('mystyle.json')
map.addLayer({...});

This would require a breaking change but I think it would greatly improve the developer experience. The current requirement to add your own logic to avoid non-deterministic layer ordering gives the API a somewhat "low level" feel.

There has been some progress introducing promises in other parts of the API (#10203), which is great. I feel like this is probably the best way forward in the long term. Thoughts?

[stevage]

I think it would be better than the current situation, yes. I'd really love a comprehensive solution that solves all the issues with "style is not finished loading", difficulty determining state, when it's safe to manipulate the map etc.

This would require a breaking change

Mapbox is, from past experience, extremely reluctant to make breaking changes - particularly in a case like this.

I think realistically, it's more likely to happen externally, maybe in mapbox-gl-utils.

setStyle(url) is in a lot of ways not very useful, anyway. It wipes all your dynamically added layers, then you have to try and add them back. So I'm likely to add a function to mapbox-gl-utils that:

• fetches the style JSON
• merges existing or desired new layers into it
• applies the merged style
• returns a promise that resolves when done.

[andycalder]

I'd really love a comprehensive solution that solves all the issues with "style is not finished loading", difficulty determining state, when it's safe to manipulate the map etc.

I think a fully promise-based API would go a long way towards solving these issues. Hopefully the switch to ES6 has brought this a little bit closer to reality. As noted in #10565:

This prevented us from using newer JS API features like Promises, but now we can start designing API’s around modern JS features.

I would enthusiastically support switching to a fully promise-based API. But yes, for now it seems like it's up to the developer, or other libraries, to work around these issues.