Script to catch unnecessary style elements

[kylebarron]

There are several style elements, like visibility: visible that are unnecessary because those are the same as the defaults. It seems advantageous and not very difficult to write a short JS script to test PRs for these elements. (It could also be added to the pre-commit hook).

My thinking is to simply traverse the JSON and check each element of each style rule against that rule's default value. So the first step I suppose is to construct a list of the style defaults.

[pathmapper]

The style defaults should all be available here:
https://raw.githubusercontent.com/mapbox/mapbox-gl-js/master/src/style-spec/reference/v8.json

e.g. https://github.com/mapbox/mapbox-gl-js/blob/master/src/style-spec/reference/v8.json#L673

[orangemug]

If it was built as a standalone lib, and exports the following definition, we could run it when opening a style in maputnik also (I guess we'd probably want to prompt the user also)

export default function (style, styleSpec) {
    // pruning code...
    return newStyle;
}

[kylebarron]

Oh good idea, I was thinking only of validating an existing style, but pruning it and returning the minimal style would be good too.

[kylebarron]

Ok I have a basic example here:
https://github.com/kylebarron/mapbox-gl-style-prune

My JS isn't great yet, so it can absolutely be improved, but it's a start that works on my osm-liberty-topo style, which is included in the repo for testing.

It checks all paint and layout properties on the style against the default property on the style spec's object.

[kylebarron]

Also I'd be happy to move it to @maputnik if you want

[orangemug]

My JS isn't great yet

It looks good to me @kylebarron

Also I'd be happy to move it to @maputnik if you want

Thanks, I think that would be good, however let's hold off on that until we have it as a part of the Maputnik codebase somewhere. My preference to adding repos to the Maputnik org is, hold off adding them until they become a little more mature/used/tested. I think it'll keep the organisation tidy in the long run.

[pathmapper]

@kylebarron 👍

Regarding:

we could run it when opening a style in maputnik also (I guess we'd probably want to prompt the user also)

I also think it would be good to integrate this in Maputnik somehow, but I'm not sure if this should run when opening a style:

• Playing around with a style and trying different setting in Maputnik could add some default values to the style (see e.g. Add styling for landcover, class==ice #71 (comment)). So there should also be an option to run the script when exporting a style.
• If the script runs when opening a style, we should prompt the user because the style could be modified (as @orangemug mentioned). I like how easy and fast it is currently to open a style in Maputnik. Such a prompt would need action from the user because there should be a choice, so the opening of the style would not be as easy and fast as before.

What about adding a checkbox "Prune style" to the export modal (which is checked by default) and do nothing when opening a style?

The current export modal:

[kylebarron]

I need to figure out how to expose it as a CLI, and also need to figure out what framework to use for basic tests.

I like @pathmapper's suggestions of it not running it when opening a style but with the option of running it when exporting it.