Safely Remove Control

[gpbmike]

Removing the draw control before the map is loaded throws errors related to missing layers.

This PR introduces a two part solution:

1. When the control is removed, stop waiting for map to load.
2. Check for source/layer presence before attempting to remove them.

fixes #572

I would love feedback.

[mcwhittemore]

@gpbmike - Thanks for this. I think this solves the bug when adding and remove Draw. I've tested the fiddle from #572 to see how adding polygons reacts when in a non set mode. There don't seem to be any errors and the data isn't added, so I think we're ok there.

Can you add a test that confirms that a if map.loaded() returns false setup.connect is not called and that then, when map.loaded() returns true setup.connect is called.

Also, can you add a test that confirms removeLayers does not error without the Draw having been fully added to the map.

[gpbmike]

@mcwhittemore sure thing. Is there an existing test file that would make sense to add to or should I create a new one?

[mcwhittemore]

Is there an existing test file that would make sense to add to or should I create a new one?

Whatever is easiest for you works for me.

[gpbmike]

For some reason I'm not able to run the test suite. It just hangs here after using npm test or yarn test. Any suggestions?

TAP version 13
# Draw.getFeatureIdsAt

[mcwhittemore]

What version of node are you using?

[gpbmike]

What version of node are you using?

v8.4.0

[mcwhittemore]

Travis is running v6.x.x - can you try in that range?

[gpbmike]

No dice. Still hanging.

mike at 3DR-000316 in ~
$ nvm use 6
Now using node v6.11.3 (npm v3.10.10)

mike at 3DR-000316 in ~
$ which node
/Users/mike/.nvm/versions/node/v6.11.3/bin/node

mike at 3DR-000316 in ~
$ cd oss/mapbox-gl-draw/

mike at 3DR-000316 in ~/oss/mapbox-gl-draw on master [!?]
$ npm --version
3.10.10

mike at 3DR-000316 in ~/oss/mapbox-gl-draw on master [!?]
$ npm test

> @mapbox/[email protected] test /Users/mike/oss/mapbox-gl-draw
> npm run lint && npm run tape


> @mapbox/[email protected] lint /Users/mike/oss/mapbox-gl-draw
> eslint --no-eslintrc -c .eslintrc index.js src test


> @mapbox/[email protected] tape /Users/mike/oss/mapbox-gl-draw
> tape -r ./test/mock-browser.js -r babel-register test/*.test.js

TAP version 13
# Draw.getFeatureIdsAt

I tried a fresh npm install with the lower npm version but that fails on the lack of gulp-sourcemaps.

[mcwhittemore]

hmmm... I'm not really sure why this would happen.

[gpbmike]

I got it working by looking at what Travis is doing. Turns out I needed to use node v6 and use yarn to install dependencies and then run tests.

Now on to the tests. I'm admittedly weak on tests. How would I go about spying on setup.connect when it's buried in a function response?

[mcwhittemore]

I spent some time vetting this today. No obvious problems. We do need to refactor tests to make it easier to test this kind of change. Thanks @gpbmike for your work here.