-
Notifications
You must be signed in to change notification settings - Fork 41
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
[Bugfix] JS error on control removal #259
Conversation
β Deploy Preview for vue-mapbox-gl-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Export Size@studiometa/vue-mapbox-gl
Unchanged@studiometa/vue-mapbox-gl
|
Codecov ReportAll modified and coverable lines are covered by tests β
Additional details and impacted files@@ Coverage Diff @@
## develop #259 +/- ##
===========================================
- Coverage 64.13% 64.00% -0.13%
===========================================
Files 22 22
Lines 2445 2445
Branches 70 70
===========================================
- Hits 1568 1565 -3
- Misses 875 878 +3
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. β View full report in Codecov by Sentry. |
π Linked issue
#249
β Type of change
π Description
When unmounting the
MapboxMap
component, we call themap.remove()
methods, which loops over all added controls and removes them. But, controls are also removed inside theuseControl
composable on unmount. This works fine with some controls but fails with theMapboxGeocoder
component: it is first removed when callingmap.remove()
and then we try to remove it a second time, which fails because the container DOM element has already been removed (see https://github.com/mapbox/mapbox-gl-geocoder/blob/66c236f1f52d69bc2e2b5b8fb3ecc5255fc46acf/lib/index.js#L396-L408).The fix is simple : we check if the map still has the control we want to remove before removing it, preventing calling the control's
onRemove
method twice.π Checklist