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

geoJSON source setData should cancel outstanding setData #6256

Closed
jdeboer-geoplan opened this issue Mar 1, 2018 · 6 comments
Closed

geoJSON source setData should cancel outstanding setData #6256

jdeboer-geoplan opened this issue Mar 1, 2018 · 6 comments

Comments

@jdeboer-geoplan
Copy link

Hi brilliant project.

I've got a small issuette.

mapbox-gl-js version:
Originally found in 0.38.0 but have updated to 0.44.1 and still see the same results.

Steps to Trigger Behavior

  1. call setdata on a geojson source with a url that takes while to return
  2. call setdata again on the source with a different url that returns first.

Expected Behavior

The map should display the second geoJSON

Actual Behavior

The layer displays the second geoJSON and then the first overwrites it.

I'm not sure a synchronous version like #2275 is what I want, I think I just want setData to abort any previous operation when it gets called again.

@jfirebaugh
Copy link
Contributor

This was supposed to be fixed in 0.44.1 with #5902. Could you please create a minimal self-contained example on jsbin.com that demonstrates the issue so we can take a look?

cc @ChrisLoer

@ChrisLoer
Copy link
Contributor

Just to clarify, the expected behavior after #5902 is that the first call should run to completion, and after it finishes, the result of the last setData call should run to completion (if there were only two calls, it'd be the second one). So you should definitely always end up with the results of the last call to setData. It's not however the case that we try to abort previous operations when a new setData call arrives -- we don't do that because in some use cases setData can be called frequently, and if we aborted existing requests we could end up with starvation.

@jdeboer-geoplan
Copy link
Author

ok, think I've put together a mcve.
Try
https://jsbin.com/kabofuz/edit?html,output

It connects to an azure web service that gives it a few geoJSON points.

Click on the "Change the geojson" button and it calls set data twice, the first call has a delay on the server side. The data sets to the second call and then reverts to the first.

Thanks, please say if my example doesn't work I've not used jsbin before.

@ChrisLoer
Copy link
Contributor

Thanks @jdeboer-geoplan! I can reproduce the problem using your jsbin, although strangely I don't see the behavior if I run the same code locally (but still hitting your azure service). I'll look into it soon.

@ChrisLoer
Copy link
Contributor

🤦‍♂️ Oops! #5902 was reverted due to a different bug and reimplemented in #5988, which unfortunately went into master just after we created the v.44 release branch. That's why I saw it work locally -- I was using a current master build. The good news is, this will be fixed in the next release. 🙂

Thanks for putting the effort into the very clear repro case... I wish I had caught that #5902 wasn't actually in v.44.1 in the first place and saved you the time.

@jdeboer-geoplan
Copy link
Author

jdeboer-geoplan commented Mar 7, 2018

Ah great I'll look forward to the next release. thanks @ChrisLoer

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

No branches or pull requests

3 participants