search

open-feature / js-sdk-contrib

OpenFeature Providers and Hooks for JavaScript
https://openfeature.dev
Apache License 2.0
35 stars 37 forks source link

Client-Side OFREP Provider doesn't type-cast flag values #1059

Open Chlor87 opened 4 weeks ago

Chlor87 commented 4 weeks ago

I'm trying to use Client-Side OFREP Provider with Flipt backend. I noticed that evaluate method doesn't perform any type conversions, just the type assertion. My question is if that's by design or can it be changed? If it can, I can try and post a PR. I also noticed that a native Flipt client performs the type conversions, hence the question.

From what I gathered, the Server-Side OFREP Provider also doesn't perform type conversions.

beeme1mr commented 3 weeks ago

Hi @Chlor87, this is by design. Do you know why Flipt needs to cast flag types?

FYI @thomaspoignant @lukas-reining @markphelps

lukas-reining commented 3 weeks ago

Do you know why Flipt needs to cast flag types?

@beeme1mr I think even though we have specified this in 1.4.1 [1], we have, with Flipt and Flagsmith, at least two providers that cast flag values that do not match and can be "safely" casted. [2] I think this is fine as long as this is what the provider considers safely casteable.

For OFREP, as it is more generic, I think this could be hard to do without creating the "JS like" type coercion mess.

Chlor87 commented 3 weeks ago

Thanks for the swift reponses! @beeme1mr it appears that Flipt only supports strings and booleans, so in order to have values of other types (numbers and objects), it attempts to do the conversions on the provider side. I'm using the React SDK on top of that which has several utility functions like useNumberFlagValue or useObjectFlagValue which always fail at the moment with OFREP. Also I need to use the remote evaluation to minimize the flag info available to the end user.

beeme1mr commented 3 weeks ago

The sdk does type validation so we potentially expose a means to define custom type casting rules. I think this should live in user land and not directly in the provider.

beeme1mr commented 4 days ago

Looping @thomaspoignant into this conversation. We might be able to solve this in the provider but I don't think we should change the SDK behavior.

toddbaert commented 3 days ago

Looping @thomaspoignant into this conversation. We might be able to solve this in the provider but I don't think we should change the SDK behavior.

Yes I think this is something providers can be opinionated about - it's up to them to decide to cast/convert, or throw TypeMismatch or even provide options to the user to do either.