-
Notifications
You must be signed in to change notification settings - Fork 90
chore(ADR): Proposal to add flag type #1746
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
e08b2a0
1226202
f1b01ef
8f4e080
b8cfc68
6ef6fce
4a5081c
06657c4
02131d1
639519a
fdd6b67
c132a88
effe9c7
a0157ca
144378e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,45 +12,44 @@ updated: 2025-08-14 | |
|
||
Currently, `flagd` has inconsistent behavior in type validation between its `Resolve<T>` and `ResolveAll` API methods. The `Resolve<T>` method validates the evaluated flag variant against the type `T` requested by the client, while `ResolveAll` validates it against the type of the `defaultVariant` specified in the flag's definition. This discrepancy can lead to situations where a flag evaluation succeeds with one method but fails with the other, depending on the evaluation context and the variant returned. This inconsistent behavior is further detailed in bug report #1481. | ||
|
||
The root cause of this issue is the absence of a dedicated, authoritative type definition for the flag itself. Instead, the type is inferred from the `defaultVariant` or API itself (`T` from `Resolve<T>`) , which is not always a reliable source of truth for all possible variants. This can lead to unexpected errors and make it difficult for developers to debug their feature flags. | ||
The root cause of this issue is the absence of a dedicated, authoritative type definition for the flag itself. Instead, the type is inferred from the `defaultVariant` or API itself (`T` from `Resolve<T>`) , which is not always a reliable source of truth for all possible variants. This issue is compounded by the planned support for code-defined defaults (as detailed in the [Support Code Default ADR](./support-code-default.md)), which allows the `defaultVariant` to be `null`. This makes it impossible to infer the flag's type from the `defaultVariant`, increasing the risk of runtime errors. | ||
|
||
|
||
## Requirements | ||
|
||
* The new `type` field in the flag definition must be optional to ensure backward compatibility. | ||
* If the `type` field is present, `flagd` must validate that all variants of the flag conform to this type during initialization. | ||
* The new `flagdType`field in the flag definition must be optional to ensure backward compatibility. | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
* If the `flagdType`field is present, `flagd` must validate that all variant values of the flag conform to this type during initialization. | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
* Type mismatches found during initialization must be reported as errors. | ||
* The `Resolve<T>` and `ResolveAll` methods must use the `type` field for validation when it is available. | ||
* The `Resolve<T>` and `ResolveAll` methods must use the `flagdType`field for validation when it is available. | ||
|
||
* The implementation must be consistent with the OpenFeature specification and the flag manifest schema. | ||
andreyturkov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
## Considered Options | ||
|
||
* **Consistent `defaultVariant` Validation:** Align the behavior of `Resolve<T>` with `ResolveAll` by making `Resolve<T>` validate the evaluated variant against the type of the `defaultVariant`. | ||
* **API Extension with Explicit Flag Type:** Introduce an optional `type` property to the flag definition to serve as the authoritative source for type validation. | ||
* **API Extension with Explicit Flag Type:** Introduce an optional `flagdType`property to the flag definition to serve as the authoritative source for type validation. | ||
|
||
|
||
## Proposal | ||
|
||
This proposal is to extend the flag definition with an optional `type` property. This approach is chosen over simply aligning the `Resolve<T>` and `ResolveAll` validation because it addresses the root cause of the type inconsistency and provides a more robust, long-term solution. | ||
This proposal is to extend the flag definition with an optional `flagdType`property. This approach is chosen over simply aligning the `Resolve<T>` and `ResolveAll` validation because it addresses the root cause of the type inconsistency and provides a more robust, long-term solution. | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
By introducing an explicit `type` field, it establishes a single source of truth for the flag's type, independent of its variants. This allows for early and consistent type validation during flag definition parsing, preventing type-related errors at runtime. | ||
By introducing an explicit `flagdType`field, it establishes a single source of truth for the flag's type, independent of its variants. This allows for early and consistent type validation during flag definition parsing, preventing type-related errors at runtime. | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
The new `type` field will be optional to maintain backward compatibility with existing flag configurations. If the field is omitted, `flagd` will treat the flag as having `object`, and no type validation will be performed against the `defaultVariant`. When the `type` field is present, `flagd` will enforce that all variants of the flag conform to the specified type. | ||
The new `flagdType`field will be optional to maintain backward compatibility with existing flag configurations. If the field is omitted, `flagd` will treat the flag as having `object`, and no type validation will be performed against the `defaultVariant`. When the `flagdType`field is present, `flagd` will enforce that all variants of the flag conform to the specified type. | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
This change will make the behavior of `flagd` more predictable and reliable. | ||
|
||
|
||
### API changes | ||
|
||
The `flagd` flag definition will be updated to include an optional `type` property. This property will be a string enum with the following possible values: `"boolean"`, `"string"`, `"number"`, and `"object"`. | ||
|
||
The `flagd` flag definition will be updated to include an optional `flagdType`property. This property will be a string enum with the following possible values: `"boolean"`, `"string"`, `"integer"`, `"float"`, and `"object"`. This aligns with the OpenFeature CLI and the flag manifest schema. | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
#### JSON Schema | ||
|
||
The following changes will be made to the `schemas/json/flags.json` file: | ||
|
||
1. A new `type` property will be added to the `flag` definition: | ||
1. A new `flagdType`property will be added to the `flag` definition: | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
```json | ||
"flag": { | ||
|
@@ -63,7 +62,8 @@ The following changes will be made to the `schemas/json/flags.json` file: | |
"enum": [ | ||
"boolean", | ||
"string", | ||
"number", | ||
"integer", | ||
"float", | ||
"object" | ||
] | ||
}, | ||
|
@@ -75,7 +75,7 @@ The following changes will be made to the `schemas/json/flags.json` file: | |
} | ||
``` | ||
|
||
2. The `booleanFlag`, `stringFlag`, `numberFlag`, and `objectFlag` definitions will be updated to enforce the `type` property: | ||
2. The `booleanFlag`, `stringFlag`, `integerFlag`, `floatFlag`, and `objectFlag` definitions will be updated to enforce the `flagdType`property: | ||
andreyturkov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
||
```json | ||
"booleanFlag": { | ||
|
@@ -97,7 +97,7 @@ The following changes will be made to the `schemas/json/flags.json` file: | |
} | ||
``` | ||
|
||
Similar changes will be made to `stringFlag`, `numberFlag`, and `objectFlag` to enforce their respective types. | ||
Similar changes will be made to `stringFlag`, `integerFlag`, `floatFlag`, and `objectFlag` to enforce their respective types. | ||
|
||
### Consequences | ||
|
||
|
@@ -114,12 +114,12 @@ Similar changes will be made to `stringFlag`, `numberFlag`, and `objectFlag` to | |
### Timeline | ||
|
||
* **Phase 1: Core Implementation** | ||
* Update the `flagd` core to support the new `type` field. | ||
* Update the `flagd` core to support the new `flagdType`field. | ||
* Implement the type validation logic. | ||
* Update the JSON schema. | ||
andreyturkov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Add unit and integration tests. | ||
* **Phase 2: SDK Updates** | ||
* Update all `flagd` SDKs to support the new `type` field. | ||
* Update all `flagd` SDKs to support the new `flagdType`field. | ||
* Update flag manifest | ||
* **Phase 3: Documentation** | ||
* Update the `flagd` documentation to reflect the changes. | ||
|
Uh oh!
There was an error while loading. Please reload this page.