Proposal: Moving Layer filter from props into style

[MichaelDanielTom]

In SymbolLayer, ModelLayer, etc. there exists a filter property that takes an expression. In v11, it is required that you update filter on Layer.swift from MapboxMaps within an updateLayer closure, As seen in this example otherwise it does nothing. Currently, setting the filter prop on any layer in rnmapbox doesn't do anything because there's no code that sets it in RNMBXLayer.swift when RNMBX_11 is true, but even if we do set it here after parsing the json and converting to an expression, it still doesn't update.

We already have the infra for diffing changes and doing everything in the updateLayer closure when changing the style, so why not add it there? From an initial test it does seem to work well in there, and we don't have to have an extra updateLayer closure in RNMBXLayer.swift, which could possibly lead to two updateLayer's per render.

Additionally, the base Layer.swift doesn't have filter in it, and same with the Android equivalent, so it's not something that applies to every layer like minZoom, maxZoom, slot, etc. have.

What do you all think? @mfazekas

[mfazekas]

@MichaelDanielTom thanks much for looking into it.

I'd prefer opening a bug with a component to reproduce and do the discussion there on how to fix this.

So right now layout and paint properties of a Layer are in style. See the style spec.js:

maps/style-spec/v8.json

Lines 1202 to 1222 in ed18ecd

	"layout": {
	"type": "layout",
	"doc": "Layout properties for the layer."
	},
	"paint": {
	"type": "paint",
	"doc": "Default paint properties for this layer."
	}
	},
	"layout": [
	"layout_fill",
	"layout_line",
	"layout_circle",
	"layout_heatmap",
	"layout_fill-extrusion",
	"layout_symbol",
	"layout_raster",
	"layout_hillshade",
	"layout_background",
	"layout_sky",
	"layout_model"

https://docs.mapbox.com/help/glossary/layout-paint-property/

For me moving filter to style is an API/interface question.
I hope we can fix the issue without changing the API/interface.

[MichaelDanielTom]

Makes sense! And yea, probably wouldn't want to change the rnmapbox api unnecessarily.