Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style spec feature_filter should dedupe values for legacy in statements #8504

Closed
dereklieu opened this issue Jul 17, 2019 · 0 comments · Fixed by #8542
Closed

Style spec feature_filter should dedupe values for legacy in statements #8504

dereklieu opened this issue Jul 17, 2019 · 0 comments · Fixed by #8542

Comments

@dereklieu
Copy link
Contributor

Motivation

A customer reported an issue migrating their styles using Studio in order to use expression syntax. Their legacy filters contained an in statement that referenced duplicate values, for example:

filter: [ 'in', 'parent_0', 'AA', 'AA', 'AB', 'AC', ... ]

This seems to be a valid legacy filter, however upon migration it fails validation for the newer syntax with error output GL JSON Invalid: Branch labels must be unique.

I propose that the convertInOp method to convert legacy filter syntax include a line to remove duplicate string or numerical values. This can only benefit users of this utility by easing migration from a looser validation requirement to a tougher one.

Design Alternatives

Alternatively, we could tighten legacy validations, but that would break lots of working maps. We could also surface a warning in GL when we detect duplicate filter values in legacy syntax, however this could be confusing as the filter would still be valid.

dereklieu pushed a commit that referenced this issue Jul 25, 2019
* Demonstrate filter conversion outputting invalid match expression

* Filter duplicate values when outputting match expressions

* Use a sorting method less likely to blow up at big O

* Tighten up sort expression
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants