JS node SDK: isFeatureEnabled returning false even if the feature is enabled
Opened this issue · 7 comments
What I wanted to do
Ask if feature A is enabled when using attributes with undefined value
What I expected to happen
To get True if feature A is enabled
What actually happened
I am getting False event though feature is enabled
Steps to reproduce
- create feature A with key
feature_a - enabled feature A for everybody
- call following method from JS node sdk:
Optimizely?.isFeatureEnabled('feature_a', 'test', { deviceOs: undefined})
You will get false... This is because if the attribute has undefined then you just stop evaluating the feature for some reason and you return false...
Imho we should not return false and continue evaluation of feature flag OR at least document this behaviour and update the TS types for that function
"@optimizely/optimizely-sdk": "4.9.1",
node version: v16.13.0
@dusan-dragon I could not try this exact scenario at my end yet but our documentation does specify what type of values attributes can take and undefined is not among them. Here is what the documentation here says
You can pass strings, numbers, Booleans, and null as custom user attribute values.
It appears that null is allowed but undefined is not.
@zashraf1985 yeah but you are linking to kind of unrelated code right? I am not talking about user context stuff here...
Here are types which says anything is allowed https://github.com/optimizely/javascript-sdk/blob/master/packages/optimizely-sdk/lib/shared_types.ts#L40
Do you see a reason why undefined should not be allowed though? I mean even if it is not allowed, should the lib just ignore that attribute instead of stopping evaluation of everything?
You are right. I sent link to the wrong version of the documentation. Here is the correct link but still says the same thing about user attributes.
You are right about the type definition. We should update that to exactly allow only the types that are allowed.
Do you see a reason why undefined should not be allowed though? I mean even if it is not allowed, should the lib just ignore that attribute instead of stopping evaluation of everything?
This is something that needs more time to look up and answer but the quickest solution for you would be to see if you can either use null instead of undefined or remove the attribute altogether when it is undefined.
I am able to create my own workaround. I am reporting this issue so it can get fixed... Also the best documentation is code itself, and if the code (types) says it accept anything 🤷♂️
Anyway, it would be nice if it would accept also undefined so we do not need to have workarounds for it
It's Great to hear that you got around it using your own technique.
I fully agree with both your points. We are adding both these points to our backlog and will try to prioritize them. The first one looks like a low hanging fruit so we might actually do it pretty soon. For the second one, we will need to dig in to the code and see whats wrong and it might take some time.
Thanks a lot for the great feedback.
This is part of a maintenance story (internal ticket FSSDK-8508) that's being broken down. We'll look to get a ticket issued for this. Thanks for contributing.