Fix choropleth example

[malwoodsantoro]

The Multiple choropleth maps example is currently broken. After further investigation, it appears that this is happening because usLayer.getGeoJSON() is not returning the data in time for it to be used in the joinData function (and is ultimately returning undefined).

I tried a few fixes, but wrapping joinData in a setTimeout function seems to give getGeoJSON() the time it needs to be used later in the function. Something strange to me is that calling usLayer.getGeoJSON() and setting it to a variable before the setTimeout function still returns undefined in joinData. I have to pass the function usLayer.getGeoJSON() rather than the variable name for it to work properly.

@katydecorah would you be able to take a look at my proposed changes? I'm wondering if there is a better way to do this without setTimeout(), perhaps using await/async in some way? Would love your thoughts.

[katydecorah]

@malwoodsantoro this is interesting 🤔

I believe that this branch is behind publisher-production which is giving us a false positive. I think the issue is that when we updated this example from using a map ID to using a styleLayer, the featureLayer is no longer calling the function loadData which kicks off the handling the data.

I created a CodePen to illustrate: https://codepen.io/katydecorah/pen/01a9493b457f97626886b4d689c96ec0?editors=0010

Here's a quick summary of what that CodePen seeks to show:

Example works when rendering a map id

• loadData and joinData are logged to the console and the data is loaded properly.

Example doesn't work with styleLayer

• loadData and joinData are not logged to the console as these functions do not appear to get rendered and so the data is not loaded properly.

---

I'm not very strong on styleLayer and what a solution could look like; @riastrad or @danswick would you happen to have any suggestions?

[malwoodsantoro]

Thanks for this, @katydecorah!

I believe that this branch is behind publisher-production

Oh, that's my bad. I believe I should be caught up now.

I now see what you mean. It looks like loadData (and then joinData) is never called because the "ready" event isn't firing:

var usLayer = L.mapbox
  .featureLayer()
  .loadURL('/mapbox.js/assets/data/us.geojson')
  .addTo(map)
  .on("ready", loadData);

With var map = L.mapbox.map("map", "mapbox.light"):

With styleLayer, "ready!" is never logged.

I realize that, with my fix, I was forcing loadData to be called inline rather than rely on the event firing as it's suppose to. This led me down the rabbit hole of seeing geojson.features return undefined and thinking it must be related to getGeoJSON() not retrieving the features by the time that code executed.

[katydecorah]

@riastrad do you have any suggestions why a map that load's a style via styleLayer would not allow map.on('ready') to be called? I created a reduced test case: https://codepen.io/katydecorah/pen/1b7cc5c4bd03b06b985ed2f49b6df543?editors=0011

I think this is related to https://github.com/mapbox/mapbox.js/issues/1327 and is possibly the reason why the data in this example is getting blocked from rendering.

[riastrad]

Thanks for the tag in, @katydecorah. Unfortunately, I don't think that this is related to https://github.com/mapbox/mapbox.js/issues/1327 at all, unfortunately as that's some warning logging that's occurring because https://github.com/mapbox/mapbox.js/pull/1317 didn't account for how styleLayer sets a tileJSON property.

If I change your minimal reproduction to using v2.3.0 (when styleLayer was first introduced) I get the same results, so I think that this is a bug that's been lurking in the L.mapbox.styleLayer object since it was implemented.

I think @malwoodsantoro might be on to something here:

thinking it must be related to getGeoJSON() not retrieving the features by the time that code executed

But I would suspect that it more has to do with the fact that L.mapbox.styleLayer extends L.tileLayer and makes two requests (1 for the style.json and 1 for the tileJSON) before it fires a custom 'ready' event, which may be mucking up Leaflet's preset tileLayer events: https://github.com/mapbox/mapbox.js/blob/6a6db371daa4ac49940948417ad2c09cf51cdcbb/src/style_layer.js#L26-L48

That's as far as I got this morning, but it might be worth poking this custom event logic & seeing if we can produce different results.

[katydecorah]

Thanks for the insight @riastrad!

@malwoodsantoro The problem with using setTimeout is that time is relative and the timer is optimize for a user who has a reliable and fast connection. For example, if I disable my cache and set my connection to slow 3G the data never loads because it takes much more time to download the initial data file.

Ideally, the example waits for the data to load before we kick off the loadData function by using on or some other similar method. (I'm trying to remember if there are other methods in Mapbox.js we could use to make sure usLayer.getGeoJSON() is loaded before running loadData... I'll keep investigating, but wanted to keep this thread warm.)

[katydecorah]

@malwoodsantoro I did some looking and found this example: https://github.com/mapbox/mapbox.js/blob/publisher-production/docs/_posts/examples/v1.0.0/0100-01-01-show-marker-as-label.html#L31-L34

Here are some suggestions:

1. Rework var usLayer = .....addTo(map); to use the pattern in the example above, where the data is loaded through featureLayer('us.gejoson') and in on the function then calls loadData(this.getGeoJSON())
2. We pass usLayer's geojson to loadData, example: function loadData(geojson).
3. Then loadData passes usLayer's geojson to joinData, example: joinData(data, geojson)

[malwoodsantoro]

@katydecorah thanks for the suggestions! Believe it is working as expected now if you wouldn't mind taking another look 🎉