Fallback for add/moveLayer when provided before-layer does not exist

[averas]

In response to discussion in #3868.

When providing a before parameter to addLayer and moveLayer referencing a layer id that does not exist the current behaviour is to add the layer second to last (since the indexOf lookup returns -1):

   const index = before ? this._order.indexOf(before) : this._order.length;
   this._order.splice(index, 0, id);

This is not intuitive and this PR will instead make a fallback to adding the layer at the end (i.e. same as without the before parameter provided).

Another option could be to throw an exception in such a case.

[mollymerp]

Thanks for your contribution @averas! I think the behavior you're suggesting makes sense, but adding a console warning (util.warnOnce('warning message') should work) explaining the behavior would be worthwhile. Could you please also add a unit test that confirms this functionality?

[jfirebaugh]

The correct approach is to emit an error event: #3208, #3879.

[averas]

@jfirebaugh Emit error only, or with fallback as well?

[averas]

To be clear, which alternative should I pursue?

1. Emit error and continue with current behaviour, i.e. add layer second to last (I really don't think so)
2. Emit error (with explanation) and fallback to adding the layer at the end
3. Emit error and bail (do nothing)

[jfirebaugh]

I'd say "Emit error and bail" -- that's consistent with what happens if a validation error occurs elsewhere.

[mourner]

@averas any updates on this PR?

[jfirebaugh]

Hey @averas -- I'm in the process of cleaning out our PR queue and it looks like there hasn't been any activity on this one recently. If you're still interested in coming back to it, please let us know and we'll be happy to help you get it to completion. Until then, I'm going to close it out. Thanks again for your contributions!