fix(style): preserve Mapbox color classes (#1957)

* fix(style): preserve Mapbox color classes

* fix(style): track applied layer stacks

* fix(style): reject legacy stacked filters

* fix(style): preserve stacked zoom ranges

* fix(style): reject legacy not-has filters

* fix(style): detect nested legacy filters

* test(style): cover modern in filters

* Address CodeRabbit review feedback

- Treat `none` as a legacy layer filter unconditionally in
  `isLegacyLayerFilter`. It has no MapLibre expression counterpart, so
  recursing into its children accepted `["none", ["has", "class"]]` and
  forwarded an invalid operator into the generated `case` color expression.
  `all`/`any` keep recursing, since both are real expression operators.
- Add a regression test covering a `none` filter wrapping an expression child.

Author: giswqs

packages/map/src/mapbox-style-import.ts

@@ -56,6 +56,7 @@ export interface MapboxStyleImportResult {
 
 /** A minimal structural view of a Mapbox GL layer, so tests need no full spec. */
 interface RawStyleLayer {
+  id?: unknown;
   type?: unknown;
   paint?: Record<string, unknown> | null;
   layout?: Record<string, unknown> | null;
@@ -107,6 +108,13 @@ function getProperty(node: unknown): string | null {
   return asString(array[1]);
 }
 
+/** Match a feature-property `get`, excluding the 3-argument object lookup. */
+function getFeatureProperty(node: unknown): string | null {
+  const array = asArray(node);
+  if (!array || array.length !== 2) return null;
+  return getProperty(array);
+}
+
 /**
  * Match the field text-field / categorized input shapes the exporter emits,
  * returning the underlying property name:
@@ -151,7 +159,7 @@ function parseColorValue(value: unknown, warnings: string[]): ParsedColor {
 
   // Unwrap the simplestyle per-feature override the exporter wraps colors in
   // (`["coalesce", ["get", key], base]`) and read the base renderer.
-  if (array[0] === "coalesce" && array.length === 3) {
+  if (array[0] === "coalesce" && array.length === 3 && getFeatureProperty(array[1]) !== null) {
     return parseColorValue(array[2], warnings);
   }
 
@@ -325,6 +333,83 @@ function parseCase(array: unknown[]): ParsedColor {
   return { mode: "rule-based", rules, color: elseColor };
 }
 
+/**
+ * Whether a top-level layer filter uses the legacy property-name syntax, such
+ * as `["==", "class", "park"]`. GeoLibre rules are MapLibre expressions, where
+ * that property must instead be `["get", "class"]`.
+ */
+function isLegacyLayerFilter(filter: unknown[]): boolean {
+  const operator = filter[0];
+  // `none` has no expression counterpart, so it is legacy whatever it wraps.
+  if (operator === "none") return true;
+  if (operator === "all" || operator === "any") {
+    return filter.slice(1).some((child) => {
+      const array = asArray(child);
+      return array !== null && isLegacyLayerFilter(array);
+    });
+  }
+  if (operator === "!") {
+    const child = asArray(filter[1]);
+    return child !== null && isLegacyLayerFilter(child);
+  }
+  if (operator === "!has") return true;
+  if (operator === "!in") return true;
+  if (operator === "in") {
+    // Modern `in` is exactly ["in", needle, haystack]. An expression-array
+    // haystack is unambiguously modern even when the needle is a string.
+    return filter.length !== 3 || (typeof filter[1] === "string" && !Array.isArray(filter[2]));
+  }
+  return (
+    ["==", "!=", ">", ">=", "<", "<="].includes(String(operator)) && typeof filter[1] === "string"
+  );
+}
+
+/**
+ * Combine a stack of filtered, flat-color Mapbox layers into GeoLibre rules.
+ * Mapbox draws later layers over earlier ones, so reverse the stack to preserve
+ * that precedence in GeoLibre's first-match-wins rule evaluator. An off else
+ * rule keeps features outside every source-layer filter hidden.
+ */
+function parseStackedLayerColors(
+  layers: RawStyleLayer[],
+  paintProperty: string,
+): ParsedColor | null {
+  if (layers.length < 2) return null;
+  const entries = layers.map((layer) => ({
+    id: asString(layer.id),
+    filter: asArray(layer.filter),
+    color: asString((layer.paint ?? {})[paintProperty]),
+    minZoom: clampZoom(layer.minzoom),
+    maxZoom: clampZoom(layer.maxzoom),
+  }));
+  if (
+    entries.some(
+      (entry) => !entry.filter || isLegacyLayerFilter(entry.filter) || entry.color === null,
+    )
+  ) {
+    return null;
+  }
+
+  const rules: NonNullable<LayerStyle["vectorRules"]> = entries.reverse().map((entry, index) => ({
+    id: `import-layer-${index}`,
+    label: entry.id ?? "",
+    filter: JSON.stringify(entry.filter),
+    color: entry.color!,
+    isElse: false,
+    ...(entry.minZoom === null ? {} : { minZoom: entry.minZoom }),
+    ...(entry.maxZoom === null ? {} : { maxZoom: entry.maxZoom }),
+  }));
+  rules.push({
+    id: "import-layer-else",
+    label: "",
+    filter: "",
+    color: DEFAULT_LAYER_STYLE.fillColor,
+    isElse: true,
+    enabled: false,
+  });
+  return { mode: "rule-based", rules };
+}
+
 /**
  * Apply a parsed color renderer to the style patch. `single`/`expression` leave
  * the flat fallback in `fillColor`; the attribute-driven modes carry the
@@ -347,7 +432,12 @@ function parseStrokeColor(value: unknown): string | null {
   // Unwrap the simplestyle per-feature override the exporter wraps line/outline
   // colors in (`["coalesce", ["get","stroke"], base]`) when simpleStyleEnabled,
   // matching parseColorValue, so the flat stroke is still recovered.
-  if (array && array[0] === "coalesce" && array.length === 3) {
+  if (
+    array &&
+    array[0] === "coalesce" &&
+    array.length === 3 &&
+    getFeatureProperty(array[1]) !== null
+  ) {
     return parseStrokeColor(array[2]);
   }
   const flat = asString(value);
@@ -510,6 +600,23 @@ function applyZoomRange(layer: RawStyleLayer, patch: Partial<Omit<LayerStyle, "l
   if (max !== null) patch.maxZoom = max;
 }
 
+/** Keep the outer layer window wide enough for every recovered stacked rule. */
+function applyStackedZoomRange(
+  layers: RawStyleLayer[],
+  patch: Partial<Omit<LayerStyle, "labels">>,
+): void {
+  const ranges = layers.map((layer) => {
+    const rawMin = clampZoom(layer.minzoom);
+    const rawMax = clampZoom(layer.maxzoom);
+    if (rawMin !== null && rawMax !== null) {
+      return { min: Math.min(rawMin, rawMax), max: Math.max(rawMin, rawMax) };
+    }
+    return { min: rawMin ?? MIN_LAYER_ZOOM, max: rawMax ?? MAX_LAYER_ZOOM };
+  });
+  patch.minZoom = Math.min(...ranges.map((range) => range.min));
+  patch.maxZoom = Math.max(...ranges.map((range) => range.max));
+}
+
 /** Build the label patch from a `symbol` layer's layout/paint. */
 function parseLabelLayer(layer: RawStyleLayer, warnings: string[]): Partial<LabelStyle> {
   const layout = layer.layout ?? {};
@@ -635,14 +742,10 @@ export function parseMapboxStyle(input: unknown): MapboxStyleImportResult {
   // stroke/radius. Track whether the color mode has been claimed.
   let colorClaimed = false;
 
-  // Only the first render layer of each type feeds the single GeoLibre style, so
-  // warn when a style stacks several (a sub-styled fill, multiple label configs)
-  // rather than dropping the extras silently.
-  for (const type of ["fill", "fill-extrusion", "line", "circle", "symbol"]) {
-    if (byType(type).length > 1) {
-      warnings.push(`The style has multiple ${type} layers; only the first was imported.`);
-    }
-  }
+  const stackedFill = parseStackedLayerColors(byType("fill"), "fill-color");
+  const stackedLine = parseStackedLayerColors(byType("line"), "line-color");
+  const stackedCircle = parseStackedLayerColors(byType("circle"), "circle-color");
+  const appliedStackTypes = new Set<string>();
 
   const [fill] = byType("fill");
   const [extrusion] = byType("fill-extrusion");
@@ -675,10 +778,11 @@ export function parseMapboxStyle(input: unknown): MapboxStyleImportResult {
     if (base !== null) patch.extrusionBase = base;
     applyZoomRange(extrusion, patch);
   } else if (fill) {
-    matchedLayerCount += 1;
+    matchedLayerCount += stackedFill ? byType("fill").length : 1;
+    if (stackedFill) appliedStackTypes.add("fill");
     patch.extrusionEnabled = false;
     const paint = fill.paint ?? {};
-    const fillColor = parseColorValue(paint["fill-color"], warnings);
+    const fillColor = stackedFill ?? parseColorValue(paint["fill-color"], warnings);
     applyColorRenderer(fillColor, patch);
     // Only claim the shared renderer when fill-color actually yielded one, so a
     // fill layer with a missing/unparseable color does not block a later
@@ -693,7 +797,8 @@ export function parseMapboxStyle(input: unknown): MapboxStyleImportResult {
     }
     const outline = parseStrokeColor(paint["fill-outline-color"]);
     if (outline) patch.strokeColor = outline;
-    applyZoomRange(fill, patch);
+    if (stackedFill) applyStackedZoomRange(byType("fill"), patch);
+    else applyZoomRange(fill, patch);
   }
 
   if (line) {
@@ -707,8 +812,12 @@ export function parseMapboxStyle(input: unknown): MapboxStyleImportResult {
     // point+line export's circle claim the renderer keeps fillColor correct; a
     // line-only layer still claims here (its fillColor is not rendered anyway).
     if (!colorClaimed && !circle) {
-      const color = parseColorValue(paint["line-color"], warnings);
+      const color = stackedLine ?? parseColorValue(paint["line-color"], warnings);
       if (color.mode && color.mode !== "single") {
+        if (stackedLine) {
+          appliedStackTypes.add("line");
+          matchedLayerCount += byType("line").length - 1;
+        }
         // Take the renderer (mode/property/stops/rules), but not the fallback
         // color: line-color's baked fallback is strokeColor (already recovered
         // above), whereas applyColorRenderer would route it into fillColor.
@@ -719,15 +828,20 @@ export function parseMapboxStyle(input: unknown): MapboxStyleImportResult {
     if (paint["line-width"] !== undefined) {
       parseLineWidth(paint["line-width"], patch, warnings);
     }
-    applyZoomRange(line, patch);
+    if (appliedStackTypes.has("line")) applyStackedZoomRange(byType("line"), patch);
+    else applyZoomRange(line, patch);
   }
 
   if (circle) {
     matchedLayerCount += 1;
     patch.pointRenderer = "single";
     const paint = circle.paint ?? {};
     if (!colorClaimed) {
-      applyColorRenderer(parseColorValue(paint["circle-color"], warnings), patch);
+      applyColorRenderer(stackedCircle ?? parseColorValue(paint["circle-color"], warnings), patch);
+      if (stackedCircle) {
+        appliedStackTypes.add("circle");
+        matchedLayerCount += byType("circle").length - 1;
+      }
       colorClaimed = true;
     }
     const radius = paint["circle-radius"];
@@ -762,7 +876,8 @@ export function parseMapboxStyle(input: unknown): MapboxStyleImportResult {
     } else {
       warnUnreadableNumber(paint["circle-stroke-width"], "point stroke width", warnings);
     }
-    applyZoomRange(circle, patch);
+    if (appliedStackTypes.has("circle")) applyStackedZoomRange(byType("circle"), patch);
+    else applyZoomRange(circle, patch);
   }
 
   if (heatmap) {
@@ -788,6 +903,19 @@ export function parseMapboxStyle(input: unknown): MapboxStyleImportResult {
     labels = parseLabelLayer(symbol, warnings);
   }
 
+  // Filtered flat-color stacks can be represented exactly as rules only when
+  // that geometry actually claims the shared renderer. Flag every other stack.
+  for (const type of ["fill", "fill-extrusion", "line", "circle", "symbol"]) {
+    if (byType(type).length < 2) continue;
+    if (appliedStackTypes.has(type)) {
+      warnings.push(
+        `The style's multiple ${type} layers were combined as rules; paint properties other than color come from the bottom-most layer.`,
+      );
+    } else {
+      warnings.push(`The style has multiple ${type} layers; only the first was imported.`);
+    }
+  }
+
   if (matchedLayerCount === 0) {
     warnings.push(
       "No fill, line, circle, heatmap, or label layers were found; nothing was imported.",