Rework javascript getProperty API to handle missing properties better - #4900
Merged
Conversation
Contributor
Author
|
I'll have a poke at CI failures this evening. |
Member
They are formatting errors you can fix with |
Contributor
Author
I even told myself "You should run that before submitting". I blame staying up until 11PM because the current token behaviour had been bothering me for long enough and doing bindings token status bars brought me close enough to the code to feel like I could do something about it. |
null is returned if the property does not exist, does not have a default or you aren't permitted to see the property.
Previously you could tell that the property was unset by if it returned the string "null", so getRawProperty gives that feature back but returning a null so you can use ?? to default it yourself. getEvaluatedProperty adds the ability to handle calculated defaults. getProperty was not made to use getEvaluatedProperty because a javascript function that knows the campaign doesn't use calculated defaults could use it to save on performance, but also that switching to evaluating by default is more likely to be a breaking change. The code for handling a missing parameter in earlier versions is awkward so it's likely that code that uses getProperty is used in campaigns where there is no default value to be reset to and all properties are explicitly set so doesn't trip the awkward behaviour. However, if this code contained objects that could be mis-parsed as macros beginning to use macros by default would be a breaking change.
fishface60
force-pushed
the
getProperty
branch
from
September 9, 2024 17:56
23a7803 to
2377222
Compare
cwisniew
approved these changes
Sep 12, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Identify the Bug or Feature request
closes #3028
Description of the Change
This changes the javascript
getPropertyfunction to look up the value in the campaign property type defaults if it's missing, and returnnullinstead of"null"if you don't have permission to get that token's properties or the campaign doesn't have that property either, for improved ergonomics of null-handling in javascript.This also adds
getRawPropertywhich returnsnullif it's not set, andgetEvaluatedPropertywhich may evaluate the value as a macro.Possible Drawbacks
If users of getProperty were checking for the value being missing with something like:
then this will now end up with value being
null.getPropertyis the most desirable name for the most common operation. It's possible that most campaigns use properties that should be evaluated so should use the shorter name, and the form ofgetPropertythat only falls back to the default value should be called something likegetUnevaluatedPropertyorgetDefaultedProperty.Release Notes
token.getPropertyreturnnullinstead of the string"null"when properties are missing or not permitted to be read.token.getPropertyreturn the default value for the token type if the value was not set on the token.token.getRawPropertyfor uses that shouldn't fall back to defaults andtoken.getEvaluatedPropertyfor uses that should use calculated defaults.This change is