Mapbox-gl-js: Return promises and/or accept callbacks for all async methods

[stevage]

Most of the maps I'm working on at the moment have the basic pattern:

• load a basemap from Mapbox (either a standard one, or slightly tweaked)
• add stuff to it with Mapbox-GL-JS, such as a GeoJSON specified by URL

But you can't add stuff to the map until it's fully loaded. So you either:

1: Wait for the map to load, then add the source by URL. This is slow, and misses out on the opportunity to do the two network requests in parallel.

1. Fetch the GeoJSON yourself (in parallel), and then add it when the map is ready. You probably end up adding D3-request or similar. Then you end up with this annoying code:

        if (map.loaded())
            map.addSource(...);
        else
            map.on('load', () => map.addSource(...));

Is there an inherent reason map can't accept new sources while it's loading?

[averas]

We just discussed this in #2792. Personally I believe there should be at least two well defined states that are exposed via the API and signalled via events, but perhaps three:

1. Map#ready (or something along that line...)
   A state the map enters as soon as it is safe to start mutating sources/layers/controls, even if the map still is busy with downloading tiles and resources. Typically as soon as the style is successfully parsed.

2. Map#loaded
   A state the map enters as soon as all necessary resources and assets, such as tiles are loaded. At this point the map is at rest.

3. Map#moving
   A state the map enters during transitions where it would make sense to hold off API calls that would interfere with user experience.

As elaborated a bit on in #2792 it might be tricky (and meaningless?) to distinguish between 2 and 3 since transitions inherently triggers tile fetching which also means that it is loading.

[stevage]

Ah, thanks. Yeah, basically I'd like to be able to add sources after Map#ready, to trigger fetching, even if they don't make it into the current render cycle.

Also, any way to clean up the is (map.loaded())... logic would be nice. Would it completely break convention to do either of these:

• map.on('load', f) immediately calls f if the map is already ready.
• map.on('load',f, true) the same, with a flag that defaults to false for current behaviour.

[lucaswoj]

Renamed this ticket to focus on the underlying challenge:

Make it easier to defer API calls until after the style loads

This is a real problem that deserves a good solution. Some ideas:

• Allow any API call before the style loads and transparently delay their action until after the style loads
• Provide a new Map#onLoad(callback) API which runs callback immediately if the map has loaded or after the style loads if not
• Add more granular Map#isStyleLoaded-like methods

[tmcw]

Some thoughts here:

Error handling would have to be async and might get harder in a bad way

Allowing access to more methods before the map's style loads would require rethought error handling: for instance, if you allow someone to call map.addLayer('foo') immediately, and then the style arrives with a layer named foo, then what would previously be an immediate exception thrown for the duplicate ID would instead appear later on, or maybe would get emitted on .on('error'.

The proposals of .on('load', fn, true) and .onLoad are promises

.on('load' is event syntax, and follows Node's expectations of events. The proposed addition of an argument or a onLoad method follows all of the conventions of a Promise, which also resolves either async or 'immediately' if the result is already available (immediately often means next tick there).

[stevage]

Hmm, good points. I'm used to working with promises, so that seemed natural to me - less familiar with .on('error') patterns. The promise pattern that combines your two comments would be:

map.addLayer('foo').catch(e => {
// oops, name collision
})

This situation actually seems a bit messy even in promise-land: map.load().then(x => map.addLayer()) misses the opportunity to run the two network requests in parallel, but Promise.all([map.load(), map.addLayer()]) ends up with a non-deterministic layer ordering. Eep.

[lucaswoj]

Related issues: https://github.com/mapbox/mapbox-gl-js/issues/4061, https://github.com/mapbox/mapbox-gl-js/issues/1794

[lucaswoj]

Methods that are candidates:

• GeoJSONSouce#setData
• Map#addLayer
• Map#addSource
• Map#fitBounds
• Map#loaded (refactor to be a promise that is fufilled when the map has loaded initially, perhaps rename)
• Map#moveLayer
• Map#remove
• Map#removeLayer
• Map#removeSource
• Map#setFilter
• Map#setLayerZoomRange
• Map#setLayoutProperty
• Map#setPaintProperty
• Map#setStyle

Methods that are candidates but will be replaced by the unified #setCamera #3583 method:

• Map#easeTo
• Map#flyTo
• Map#jumpTo
• Map#panBy
• Map#rotateTo (?)
• Map#setBearing (?)
• Map#setZoom (?)
• Map#zoomIn
• Map#zoomOut
• Map#zoomTo
• Map#zoomTo

[gertcuykens]

The only thing I want to add is don't bother with callbacks so you don't need to make the api more complicated than necessary. Using callbacks instead of promises should be avoided unless somebody can convince the majority, which I hope that doesn't happen :) There are plenty of polyfils for promises if a user wants to go back to the stone age :P

[fergardi]

Any progress on this one? How can I know when my panBy or easeTo are finished? Can we use promises now?

[mourner]

How can I know when my panBy or easeTo are finished

You can subscribe to the moveend event:

map.easeTo(...).once('moveend', () => { ... });

[soal]

I've created a library for this case: map-promisified.
How it works:
It wraps asynchronous methods with functions that return promises. Internally, when you call promisified method, it passes a unique event id in eventData map method argument and listens to all events that methods cause. A unique id is necessary to find out that the event is caused by certain map method. After all events with registered unique id are fired, the function returns promise.

I have two questions concerning this issue:

1. Can I use the idle event for this purpose? Specifically, does idle fired when all transitions caused by, for example, panTo is finished?
2. Can this approach be used in mapbox-gl-js internally?

Thank you!

[ryanhamley]

@soal The idle event is fired when

All requested tiles are loaded
No transitions are in progress
No camera animations are in progress

So yes, it should fire once all panTo transitions are finished. The original PR is https://github.com/mapbox/mapbox-gl-js/pull/7625. Adding promises to the library is not on our roadmap.

[barraponto]

I wanted to be able to say something like map.addLayer(...). then((layer) => {...}). But I would settle for an event fired when a layer is added -- regardless of whether it needed to load a source from a url or not.

[asheemmamoowala]

@barraponto This ticket is tracking the first part of your request.

I would settle for an event fired when a layer is added -- regardless of whether it needed to load a source from a url or not.

Could you open a separate tickets for the above. With some additional information on how you would expect this to work and how you would like to use it.

[backspaces]

I've been using:

function mapLoad(map) {
    return new Promise((resolve, reject) => {
        map.on('load', () => resolve())
    })
}

Which seems to be working. Adding a tag after the map would make it general for all map events. Is there any reason this sort of thing would fail in mapbox? Tnx!

[fc]

Any idea when the setFilter promise may be on the roadmap?

The current hack I'm using is a combination of using the idle and render events which works about 90-95% of the time. But, the problem is that one of the layers I'm filtering is transparent which doesn't seem to trigger the render events but I need to query the _filtered_ transparent layers while the map is loading. I can add an extra 1 second delay but it seems there's no guarantee or way for me to know if setFilter has been truly completed.

is there any other ways to figure out if filtering is completed? I tried inspecting some of the private member variables but wasn't super clear.