feat: dependent features

[kwasniew]

About the changes

Spec for dependent features

Data structure for dependent features:

• depend on parent being enabled: {feature: 'parent'}
• depend on parent being disabled (useful for kill-switches): {feature: 'parent', enabled: false}
• depend on parent resolving to a value/variant: {feature: 'parent', variants: ['parentVariant', 'anotherParentVariant']}

We're deliberately choosing feature as a key to have an option for other types of dependencies e.g. strategies

Most of the code has the same activation strategy with a strategy variant. Enabled tests and variant tests are almost identical (only description and expected result are different).

Interesting cases:

• you can depend on many features (with AND operator between them)
• when you depend on a parent feature variant you can specify a list of options that you'd like to match against (OR operator between variants)
• we don't support transitive dependencies, only direct dependencies

Reference implementation passing spec tests: Unleash/unleash-client-node#520

Important files

Discussion points

[sighphyre]

Nice, that's what I would have done too, I think this is correct. This is a big decision though. It means stale caches and/or cycling out your api key can potentially change the state of a toggle. Leaving this here for visibility in case someone has a different opinion

[kwasniew]

Glad you're confirming my confirmation bias :) As you said it's up for a discussion.

[FredrikOseberg]

This is tricky. Since you can filter by tags it's technically possible to end up in a situation where the parent is not present. We'll need to implement safeguards in the SDK to warn you if this happens, but I think the behavior here is correct. If you depend on a feature toggle that you have not fetched, there is no way for us to know the evaluation result of that toggle and we can not determine that the child should be enabled.

[FredrikOseberg]

I guess this is where a feature toggle type would come in handy @kwasniew

[kwasniew]

@FredrikOseberg I will try to add a warning when you try to depend on a feature that you don't have in the SDK

[kwasniew]

@FredrikOseberg can you elaborate on a feature toggle type?

[FredrikOseberg]

Yes. Like we talked about in the meeting, some way to mark a toggle as dependable, we discussed the possibility of having a new feature toggle type called Dependable Toggle. If we had that, we could circumvent the filtering in the API and say that "No, we don't care that you are filtering by tags, you'll get all the dependable toggles in this project regardless".

[kwasniew]

@FredrikOseberg added reference impl for warnings: Unleash/unleash-client-node@1ed0994

[kwasniew]

Now I remember the dependent feature markers we talked about :) We definitely need to keep it in mind. For this spec though I think we can proceed without this concept since it will only affect the client API not this spec.

[FredrikOseberg]

Agree. This spec just surfaced the problem, so I just mentioned it here.

[sighphyre]

I think it's worth including some tests that also depend on context properties, e.g. a constraint. Based on the node implementation the desired behaviour appears to be that the context is reused for the parent check as well. Worth enforcing here I think

[kwasniew]

@sighphyre addressed all your points :)

[sighphyre]

What a legend. This looks rather nice

I think others should weigh in but LGTM

[daveleek]

This operates on returned variant results I take it? Do default-variants count as []?

[kwasniew]

with variants: [] you don't check variants, only parent being enabled. It's the same as not providing variants at all.

with variants: ['disabled'] you check default variant.

This is reference impl: parent.variants.includes(this.getVariant(parent.feature, context).name)

If we think that it's a good idea to do it this way I can go ahead and add it to the spec.

[daveleek]

Ah, great! So "variants": [ ] is the same as not including the variants property in the dependency

[daveleek]

I like it!