mapbox / mapbox/mapbox-gl-draw
Layers using source of `mapbox-gl-draw-cold` or `mapbox-gl-draw-hot` should be remove on `onRemove`
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 1.1k
- Forks
- 612
- Avg merge
- 8d 9h
- Merged PRs (30d)
- 5
Description
Hi there,
As you know, Draw provides sources named `mapbox-gl-draw-cold` and `mapbox-gl-draw-hot`.
We render these sources in a `fill-extrusion` layer and use them to represent the polygons drawn by Draw as 3D polygons.
We understand that drawing the `mapbox-gl-draw-cold` and `mapbox-gl-draw-hot` sources in a custom layer in this way is an expected usage.
However, the current implementation of `onRemove` in Draw does not take care of cleaning up layers that use these sources. If Draw's `onRemove` is executed without removing those layers, a runtime error will occur at the point of `removingSource` `mapbox-gl-draw-cold` or `mapbox-gl-draw-hot` because there is layer that is still using those sources.
https://github.com/mapbox/mapbox-gl-draw/blob/main/src/setup.js#L111-L126
If the user were to intervene in this lifecycle to remove these layers, we would probably have to extend Draw to implement our own onRemove, which would be a large implementation compared to what we want to do.
So I think these layers should be deleted by Draw's `onRemove` with the following code added. (It's just an idea, unexamined.)
```typescript
const style = ctx.map.getStyle();
const allLayers = style ? style.layers : undefined;
if (allLayers) {
for (const layer of allLayers) {
if (layer.source === Constants.sources.COLD || layer.source === Constants.sources.HOT) {
ctx.map.removeLayer(layer.id);
}
}
}
```
What do you think about the above proposal?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/setup.js around lines 111-126 and trace how onRemove cleans up the map sources. Reproduce the case with fill-extrusion layers using mapbox-gl-draw-cold and mapbox-gl-draw-hot, then determine the appropriate layer cleanup behavior and verify that removing Draw no longer triggers a source-in-use runtime error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100