Flagsmith doesn't use boolean defaults and uses enablement status instead of flag value
Introduction
Hey team, we've began using Flagsmith in scenarios other than the most common or basic, specifically creating a boolean flag that can be overriden in certain cenarios for part of the clients based off a segment.
Basically the flag is usually a truthy value, but a segment override that will be edited over time may decrease the amount of clients that receive true and will now get a false value. Kinda like a soft phase-out of a feature.
While building that we noticed that the provider didn't take into account falsey default values in boolean flags, using a fixed truthy value in the code, and the evaluation for these boolean values was based off their existance/enablement, and not their actual value, preventing us from using these overrides that change the value, not the enabled status.
I took the liberty to aside from reporting this, also preparing a reproducible environment and a PR fix, but please feel free to discuss and opinate on the issue or solution.
Links
Reproducible Example: https://github.com/uriell/openfeature-flagsmith-boolean-issues Pull Request: https://github.com/open-feature/js-sdk-contrib/pull/1026
Thanks @uriell, circling in the Flagsmith team.
@dabeeeenster @matthewelwell
Something worth noting: Flags in Flagsmith don't require a value to be created, only a name and their enabled status. Therefore my PR in its current state would bring breaking changes to clients using flags that have no value set and rely on enabled.
I do however, think this proposal is still relevant since other OpenFeature SDKs for Flagsmith, like GoLang allow value to be used for booleans instead of enabled (at the developer's discretion). So there are two proposed scenarios:
- Current, and probably default: Boolean flags evaluate based off
enabledstatus: a. Overrides are not possible, values and default values are never used[^1]; useValueForBooleansin constructor: Boolean flags evaluate based offenabled,valueanddefaultValue: a. When a flag doesn't exist, it should returnundefinedand therefore usedefaultValue; b. Whenenabledistrue, but a flagvalueis not set, it will be falsey until explicitly set totruein flagsmith[^2]; c. Whenenabledistrue, the flagvalueshould be returned; d. Whenenabledisfalse, should the flagvalueordefaultValuebe returned?[^3];
[^1]: Currently, when a flag doesn't exist, flagsmith's hasFeature will return false, and since it is the correct expected type, it will be returned to the client without using the specified defaultValue;
[^2]: This is Flagsmith's getValue current behavior when a flag returns empty, and since it is the correct expected type, it will be returned to the client without using the specified defaultValue;
[^3]: I brought up this discussion since in GoLang's Flagsmith Provider, defaultValue is used when the flag has enabled set to false, and I'm not sure this would work for this client since hasFeature can return false both for missing flags and for flags that have enabled set to false, being slightly ambiguous;
@dabeeeenster @matthewelwell any ideas on this? Maybe this could be a configuration thing?