fix: do not fail style validation on warnings (#7941)

* Initial commit, needs the other PR to complete.

* Add test

* Update changelog

* Improve import type

* Remove comment

* Remove unneeded test

* Remove more tests

* Remove comment

* Update test

* Remove comment

* Remove tests keep coverage

* Update tests

* Move import

* Consolidate validation error types.

* Remove dead code.

* Remove dead code.

* Make the source validation more robust.

* Update build size

Author: HarelM

CHANGELOG.md

@@ -1,10 +1,13 @@
 ## main
 ### ✨ Features and improvements
+- Validate the terrain passed to `map.setTerrain`, which was previously applied unchecked ([#7941](https://github.com/maplibre/maplibre-gl-js/pull/7941)) (by [@HarelM](https://github.com/HarelM))
 - Improve runtime error warnings to point at the offending style location (e.g. `layers[3].paint.line-color`, `layers[3].filter`) instead of just logging the bare error message ([#7869](https://github.com/maplibre/maplibre-gl-js/pull/7869)) (by [@CommanderStorm](https://github.com/CommanderStorm))
 - ⚠️ Interpolate the light position in spherical coordinates instead of cartesian ones, so that a transition keeps its radial distance. ([#7919](https://github.com/maplibre/maplibre-gl-js/pull/7919)) (by [@HarelM](https://github.com/HarelM))
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
+- Log style validation warnings instead of treating them as errors, so that a filter mixing deprecated and expression syntax no longer aborts the style load and blanks the map ([#7941](https://github.com/maplibre/maplibre-style-spec/pull/7941)) (by [@HarelM](https://github.com/HarelM))
+- Validate `raster-dem` sources passed to `map.addSource`, which were previously skipped. Stop a source type the style spec has no schema for, such as one registered with `addSourceType`, from failing the whole style. Previously only `canvas` was let through ([#7941](https://github.com/maplibre/maplibre-gl-js/pull/7941)) (by [@HarelM](https://github.com/HarelM))
 - _...Add new stuff here..._
 
 ## 6.0.0-21

package-lock.json

@@ -1,12 +1,6 @@
-import {latest as styleSpec} from '@maplibre/maplibre-gl-style-spec';
-
 import {sphericalToCartesian} from '../util/util.ts';
 import {Evented} from '../util/evented.ts';
-import {
-    validateStyle,
-    validateLight,
-    emitValidationErrors
-} from './validate_style.ts';
+import {validateStyle, validateAndEmit, type Validator} from './validate_style.ts';
 import {getProperties, type LightProps, type LightPropsPossiblyEvaluated} from './light_properties.g.ts';
 
 import type {vec3} from 'gl-matrix';
@@ -44,7 +38,7 @@ export class Light extends Evented {
     }
 
     setLight(light?: LightSpecification, options: StyleSetterOptions = {}): void {
-        if (this._validate(validateLight, light, options)) {
+        if (this._validate(validateStyle.light, light, options)) {
             return;
         }
 
@@ -70,18 +64,7 @@ export class Light extends Evented {
         this.properties = this._transitioning.possiblyEvaluate(parameters);
     }
 
-    _validate(validate: Function, value: unknown, options?: {
-        validate?: boolean;
-    }): boolean {
-        if (options?.validate === false) {
-            return false;
-        }
-
-        return emitValidationErrors(this, validate.call(validateStyle, {
-            value,
-            // Workaround for https://github.com/mapbox/mapbox-gl-js/issues/2407
-            style: {glyphs: true, sprite: true},
-            styleSpec
-        }));
+    _validate(validate: Validator, value: unknown, options?: StyleSetterOptions): boolean {
+        return validateAndEmit(this, validate, {value}, options);
     }
 }

src/style/light.ts

@@ -1,11 +1,9 @@
 import {type PossiblyEvaluated, TRANSITION_SUFFIX, Transitionable, type Transitioning, type TransitionParameters} from './properties.ts';
 import {Evented} from '../util/evented.ts';
 import {EvaluationParameters} from './evaluation_parameters.ts';
-import {emitValidationErrors, validateSky, validateStyle} from './validate_style.ts';
-import {extend} from '../util/util.ts';
-import {latest as styleSpec} from '@maplibre/maplibre-gl-style-spec';
-import {type Mesh} from '../render/mesh.ts';
+import {validateStyle, validateAndEmit, type Validator} from './validate_style.ts';
 import {getProperties, type SkyProps, type SkyPropsPossiblyEvaluated} from './sky_properties.g.ts';
+import type {Mesh} from '../render/mesh.ts';
 import type {SkySpecification} from '@maplibre/maplibre-gl-style-spec';
 import type {StyleSetterOptions} from './style.ts';
 
@@ -29,7 +27,7 @@ export class Sky extends Evented {
     }
 
     setSky(sky?: SkySpecification, options: StyleSetterOptions = {}): void {
-        if (this._validate(validateSky, sky, options)) return;
+        if (this._validate(validateStyle.sky, sky, options)) return;
 
         sky ||= {
             'sky-color': 'transparent',
@@ -65,16 +63,8 @@ export class Sky extends Evented {
         this.properties = this._transitioning.possiblyEvaluate(parameters);
     }
 
-    _validate(validate: Function, value: unknown, options: StyleSetterOptions = {}): boolean {
-        if (options?.validate === false) {
-            return false;
-        }
-        return emitValidationErrors(this, validate.call(validateStyle, extend({
-            value,
-            // Workaround for https://github.com/mapbox/mapbox-gl-js/issues/2407
-            style: {glyphs: true, sprite: true},
-            styleSpec
-        })));
+    _validate(validate: Validator, value: unknown, options: StyleSetterOptions = {}): boolean {
+        return validateAndEmit(this, validate, {value}, options);
     }
 
     /**

src/style/sky.ts

@@ -51,6 +51,12 @@ function createGeoJSONSource(): GeoJSONSourceSpecification {
     };
 }
 
+const mixedLegacyAndExpressionFilter = [
+    'all',
+    ['==', ['get', 'class'], 'rail'],
+    ['in', 'name', '']
+] as any as FilterSpecification;
+
 const getStubMap = () => new StubMap() as any;
 
 function createStyle(map = getStubMap()) {
@@ -227,6 +233,25 @@ describe('Style.loadJSON', () => {
         expect(style.serialize()).toEqual(createStyleJSON());
     });
 
+    test('loads a style whose filter mixes legacy and expression syntax, warning instead of blanking the map', async () => {
+        const style = new Style(getStubMap());
+        const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+        const errorSpy = vi.fn();
+        style.on('error', errorSpy);
+
+        style.loadJSON(createStyleJSON({
+            sources: {geojson: createGeoJSONSource()},
+            layers: [{id: 'symbol', type: 'symbol', source: 'geojson', filter: mixedLegacyAndExpressionFilter}]
+        }));
+
+        await style.once('style.load');
+
+        expect(errorSpy).not.toHaveBeenCalled();
+        expect(style.getLayer('symbol')).toBeTruthy();
+        expect(style.getFilter('symbol')).toEqual(mixedLegacyAndExpressionFilter);
+        expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Mixing deprecated filter syntax with expression syntax'));
+    });
+
     test('fires "dataloading" (synchronously)', () => {
         const style = new Style(getStubMap());
         const spy = vi.fn();
@@ -3037,6 +3062,20 @@ describe('Style.setFilter', () => {
         expect(spy.mock.calls[0][1]['layers'][0].id).toBe('symbol');
         expect(spy.mock.calls[0][1]['layers'][0].filter).toBe('notafilter');
     });
+
+    test('warns instead of emitting for a filter that mixes legacy and expression syntax', async () => {
+        const style = createStyle();
+        await style.once('style.load');
+        const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+        const errorSpy = vi.fn();
+        style.on('error', errorSpy);
+
+        style.setFilter('symbol', mixedLegacyAndExpressionFilter);
+
+        expect(style.getFilter('symbol')).toEqual(mixedLegacyAndExpressionFilter);
+        expect(errorSpy).not.toHaveBeenCalled();
+        expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Mixing deprecated filter syntax with expression syntax'));
+    });
 });
 
 describe('Style.setLayerZoomRange', () => {

src/style/style.test.ts

@@ -16,10 +16,10 @@ import {ResourceType} from '../util/request_manager.ts';
 import {browser} from '../util/browser.ts';
 import {now} from '../util/time_control.ts';
 import {Dispatcher} from '../util/dispatcher.ts';
-import {validateStyle, emitValidationErrors as _emitValidationErrors} from './validate_style.ts';
+import {validateStyle, validateStyleAndEmit, validateAndEmit, emitValidationErrors, SPEC_SOURCE_TYPES} from './validate_style.ts';
 import {type QueryRenderedFeaturesOptions, type QueryRenderedFeaturesOptionsStrict, type QueryRenderedFeaturesResults, type QueryRenderedFeaturesResultsItem, type QuerySourceFeatureOptions, queryRenderedFeatures, queryRenderedSymbols, querySourceFeatures} from '../source/query_features.ts';
 import {TileManager} from '../tile/tile_manager.ts';
-import {latest as styleSpec, derefLayers, emptyStyle, diff as diffStyles, type DiffCommand} from '@maplibre/maplibre-gl-style-spec';
+import {derefLayers, emptyStyle, diff as diffStyles, type DiffCommand} from '@maplibre/maplibre-gl-style-spec';
 import {getGlobalWorkerPool} from '../util/global_worker_pool.ts';
 import {rtlMainThreadPluginFactory} from '../source/rtl_text_plugin_main_thread.ts';
 import {RTLPluginLoadedEventName} from '../source/rtl_text_plugin_status.ts';
@@ -34,15 +34,6 @@ import type {StyleLayer} from './style_layer.ts';
 import type {MapGeoJSONFeature, GeoJSONFeature} from '../util/vectortile_to_geojson.ts';
 import type Point from '@mapbox/point-geometry';
 
-// We're skipping validation errors with the `source.canvas` identifier in order
-// to continue to allow canvas sources to be added at runtime/updated in
-// smart setStyle (see https://github.com/mapbox/mapbox-gl-js/pull/6424):
-const emitValidationErrors = (evented: Evented, errors?: ReadonlyArray<{
-    message: string;
-    identifier?: string;
-}> | null) =>
-    _emitValidationErrors(evented, errors?.filter(error => error.identifier !== 'source.canvas'));
-
 import type {Map} from '../ui/map.ts';
 import type {IReadonlyTransform, ITransform} from '../geo/transform_interface.ts';
 import type {StyleImage} from './style_image.ts';
@@ -478,7 +469,7 @@ export class Style extends Evented<MapEventType> {
 
     _load(json: StyleSpecification, options: StyleSwapOptions & StyleSetterOptions, previousStyle?: StyleSpecification): void {
         let nextState = options.transformStyle ? options.transformStyle(previousStyle, json) : json;
-        if (options.validate && emitValidationErrors(this, validateStyle(nextState))) {
+        if (options.validate && validateStyleAndEmit(this, nextState)) {
             return;
         }
 
@@ -505,7 +496,8 @@ export class Style extends Evented<MapEventType> {
 
         this.sky = new Sky(this.stylesheet.sky);
 
-        this.map.setTerrain(this.stylesheet.terrain ?? null);
+        // The stylesheet's terrain was already validated as part of the style itself.
+        this.map.setTerrain(this.stylesheet.terrain ?? null, {validate: false});
 
         this.fire(new MapStyleDataEvent('data'));
         this.fire(new MapStyleLoadEvent());
@@ -879,7 +871,7 @@ export class Style extends Evented<MapEventType> {
         const serializedStyle =  this.serialize();
         nextState = options.transformStyle ? options.transformStyle(serializedStyle, nextState) : nextState;
         const validate = options.validate ?? true;
-        if (validate && emitValidationErrors(this, validateStyle(nextState))) return false;
+        if (validate && validateStyleAndEmit(this, nextState)) return false;
 
         nextState = clone(nextState);
         nextState.layers = derefLayers(nextState.layers);
@@ -1038,8 +1030,7 @@ export class Style extends Evented<MapEventType> {
             throw new Error(`The type property must be defined, but only the following properties were given: ${Object.keys(source).join(', ')}.`);
         }
 
-        const builtIns = ['vector', 'raster', 'geojson', 'video', 'image'];
-        const shouldValidate = builtIns.includes(source.type);
+        const shouldValidate = SPEC_SOURCE_TYPES.has(source.type);
         if (shouldValidate && this._validate(validateStyle.source, `sources.${id}`, source, null, options)) return;
         if (this.map?._collectResourceTiming) (source as any).collectResourceTiming = true;
         const tileManager = this.tileManagers[id] = new TileManager(id, source, this.dispatcher);
@@ -1777,18 +1768,13 @@ export class Style extends Evented<MapEventType> {
         }
     }
 
-    _validate(validate: Validator, key: string, value: any, props: any, options: {
-        validate?: boolean;
-    } = {}): boolean {
-        if (options?.validate === false) {
-            return false;
-        }
-        return emitValidationErrors(this, validate.call(validateStyle, extend({
+    _validate(validate: Validator, key: string, value: any, props: any, options: StyleSetterOptions = {}): boolean {
+        return validateAndEmit(this, validate, {
             key,
             style: this.serialize(),
             value,
-            styleSpec
-        }, props)));
+            ...props
+        }, options);
     }
 
     _remove(mapRemoved: boolean = true): void {

src/style/style.ts

@@ -1,12 +1,7 @@
 import {filterObject} from '../util/util.ts';
 
-import {createVisibilityExpression, featureFilter, latest as styleSpec, supportsPropertyExpression} from '@maplibre/maplibre-gl-style-spec';
-import {
-    validateStyle,
-    validateLayoutProperty,
-    validatePaintProperty,
-    emitValidationErrors
-} from './validate_style.ts';
+import {createVisibilityExpression, featureFilter, supportsPropertyExpression} from '@maplibre/maplibre-gl-style-spec';
+import {validateStyle, validateAndEmit, type Validator} from './validate_style.ts';
 import {Evented, ErrorEvent} from '../util/evented.ts';
 import {Layout, Transitionable, type Transitioning, type Properties, PossiblyEvaluated, PossiblyEvaluatedPropertyValue, TRANSITION_SUFFIX} from './properties.ts';
 
@@ -262,7 +257,7 @@ export abstract class StyleLayer extends Evented {
             return;
         }
 
-        if (value !== null && value !== undefined && this._validate(validateLayoutProperty, `layers.${this.id}.layout.${name}`, name, value, options))  return;
+        if (value !== null && value !== undefined && this._validate(validateStyle.layoutProperty, `layers.${this.id}.layout.${name}`, name, value, options))  return;
 
         this._unevaluatedLayout.setValue(name, value);
     }
@@ -290,7 +285,7 @@ export abstract class StyleLayer extends Evented {
             return false;
         }
 
-        if (value !== null && value !== undefined && this._validate(validatePaintProperty, `layers.${this.id}.paint.${name}`, name, value, options)) return false;
+        if (value !== null && value !== undefined && this._validate(validateStyle.paintProperty, `layers.${this.id}.paint.${name}`, name, value, options)) return false;
 
         if (name.endsWith(TRANSITION_SUFFIX)) {
             this._transitionablePaint.setTransition(name.slice(0, -TRANSITION_SUFFIX.length), (value as any) || undefined);
@@ -381,19 +376,13 @@ export abstract class StyleLayer extends Evented {
         });
     }
 
-    _validate(validate: Function, key: string, name: string, value: unknown, options: StyleSetterOptions = {}): boolean {
-        if (options?.validate === false) {
-            return false;
-        }
-        return emitValidationErrors(this, validate.call(validateStyle, {
+    _validate(validate: Validator, key: string, name: string, value: unknown, options: StyleSetterOptions = {}): boolean {
+        return validateAndEmit(this, validate, {
             key,
             layerType: this.type,
             objectKey: name,
-            value,
-            styleSpec,
-            // Workaround for https://github.com/mapbox/mapbox-gl-js/issues/2407
-            style: {glyphs: true, sprite: true}
-        }));
+            value
+        }, options);
     }
 
     is3D(): boolean {

src/style/style_layer.ts

@@ -1,7 +1,8 @@
 import {StyleLayer} from '../style_layer.ts';
+import {ValidationError} from '@maplibre/maplibre-gl-style-spec';
 import type {Map} from '../../ui/map.ts';
-import {type mat4} from 'gl-matrix';
-import {type LayerSpecification} from '@maplibre/maplibre-gl-style-spec';
+import type {mat4} from 'gl-matrix';
+import type {LayerSpecification} from '@maplibre/maplibre-gl-style-spec';
 import type {CustomLayerProjectionData, RendererProjectionData} from '../../geo/projection/projection_data.ts';
 
 /**
@@ -292,28 +293,22 @@ export interface CustomLayerInterface {
     onRemove?(map: Map, gl: WebGL2RenderingContext): void;
 }
 
-export function validateCustomStyleLayer(layerObject: CustomLayerInterface): Array<{message: string}> {
-    const errors: Array<{message: string}> = [];
+export function validateCustomStyleLayer(layerObject: CustomLayerInterface): ValidationError[] {
+    const errors: ValidationError[] = [];
     const id = layerObject.id;
 
     if (id === undefined) {
-        errors.push({
-            message: `layers.${id}: missing required property "id"`
-        });
+        errors.push(new ValidationError(`layers.${id}`, null, 'missing required property "id"'));
     }
 
     if (layerObject.render === undefined) {
-        errors.push({
-            message: `layers.${id}: missing required method "render"`
-        });
+        errors.push(new ValidationError(`layers.${id}`, null, 'missing required method "render"'));
     }
 
     if (layerObject.renderingMode &&
         layerObject.renderingMode !== '2d' &&
         layerObject.renderingMode !== '3d') {
-        errors.push({
-            message: `layers.${id}: property "renderingMode" must be either "2d" or "3d"`
-        });
+        errors.push(new ValidationError(`layers.${id}`, null, 'property "renderingMode" must be either "2d" or "3d"'));
     }
 
     return errors;