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
High level description of type system, assertions vs. conversion #5701
Conversation
2f94ffd
to
aae567f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. One suggestion inline.
docs/pages/style-spec.js
Outdated
in certain situations, the SDK may be unable to automatically determine the expected | ||
result type of a data expression from surrounding context. For example, it is not clear | ||
whether the expression <code>["<", ["get", "a"], ["get", "b"]]</code> is attempting | ||
to compare strings or numbers. In situations like this, you must use one of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"must" could be misleading, since using a conversion rather than an assertion is also an option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
In situations like this, the simplest option is to use type assertions:
["<", ["number", ["get", "a"]], ["number", ["get", "b"]]]
. A type assertion checks that the feature data actually matches the expected type of the data expression. If this check fails, it produces an error and causes the whole expression to fall back to the default value for the property being defined. The assertion operators are ...Another option is to use a type conversion. Expressions only perform one kind of implicit type conversion...
This repeats the description of runtime type checking from the previous paragraph, but I think the redundancy is worthwhile in order to explicitly state what an assertion does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, not convinced about this change. I think we should keep the use cases clearly separated here: conversion is for when you have one type, but need another. Assertion is for when the implementation needs you to tell it what type you have. If you are using a conversion, then you're not in a situation where the implementation is unable to automatically determine the type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are using a conversion, then you're not in a situation where the implementation is unable to automatically determine the type.
You could be in such a situation -- e.g., say you want to do ["==", ["id"], ["get", "name"]]
. Depending on your data, you really might want to use conversion rather than assertion as your way out of the type checking error.
That said, I'm 👍 for focusing on assertions as the primary way to deal with this, since, though conversions can be used in this way, their typical use cases are much broader. I still think the word "must" is misleading, and also that we should include an explicit description of assertions' behavior.
aae567f
to
909186b
Compare
Per #5219, add documentation regarding the expression type system.