Skip to content

Conversation

@erdirowlands
Copy link
Contributor

@erdirowlands erdirowlands commented Oct 16, 2024

What

Adds a new optional prop called onFlagNotFound, which is a callback that gets triggered whenever a variation is requested and the flag cannot be found. This prop receives two arguments:

flagNotFound : A typeDefaultVariationEventPayload from the Javascript SDK containing the flag identifier and the default variation that was served.

loading: A boolean that indicates whether the SDK was still initializing at the time the variation call was made and the flag was not found.

The mechanism for achieving this is listening to the new Javascript SDK event (as of 1.29.0) ERROR_DEFAULT_VARIATION_RETURNED which is fired by the Javascript SDK when a call to variation has returned the default variation.

Why

Inform users when the SDK has served a default variation, for example, due to a flag typo, misconfiguration, or the SDK not being fully initialized. For cases where the SDK is not initialized, the user may choose not to care as this is the default behaviour in async mode, and the loading argument can be used to filter those out.

Testing

Added new test suite for FFContext

Manual
Using useFeatureFlag hook:

  • async disabled
    • Non-existent flag: onFlagNotFound executes when the SDK has finished initialising
    • Flag exists: onFlagNotFound does not execute after the SDK initialises
  • async enabled
    • Non-existent flag: onFlagNotFound executes immediately, and again when the SDK initializes.
    • Existing flag: onFlagNotFound executes immediately, but does not execute after the SDK initiailises

@erdirowlands erdirowlands marked this pull request as draft October 16, 2024 13:52
const [clientInstance, setClientInstance] =
useState<FFContextValue['client']>()

// Use a reference to keep track of the latest loading state, allowing access to its current value
Copy link
Contributor Author

@erdirowlands erdirowlands Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using the loading state and passing it to the new onFlagNotFound option. But it would still be true and not false when the SDK initialised and onFlagNotFound was called. I think this is due to the state being updated async, so this is what I went with as an alternative.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is that your handler is set in stone at the point it's created and so has the value of the state. This is because it's being defined in the useEffect, which isn't being recalled when the loading state changes. The ref is a cleaver way of getting the right value without triggering renders 👍🏻

Copy link
Collaborator

@knagurski knagurski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor stuff

const [clientInstance, setClientInstance] =
useState<FFContextValue['client']>()

// Use a reference to keep track of the latest loading state, allowing access to its current value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is that your handler is set in stone at the point it's created and so has the value of the state. This is because it's being defined in the useEffect, which isn't being recalled when the loading state changes. The ref is a cleaver way of getting the right value without triggering renders 👍🏻

@erdirowlands erdirowlands marked this pull request as ready for review October 17, 2024 11:20
Apply suggestions from code review

Co-authored-by: Kevin Nagurski <Kevin.nagurski@harness.io>
@erdirowlands erdirowlands force-pushed the FFM-12129 branch 2 times, most recently from c6b9bd7 to a5721a5 Compare October 17, 2024 11:45
@erdirowlands erdirowlands force-pushed the FFM-12129 branch 2 times, most recently from a5e0f2a to ee6acba Compare October 17, 2024 12:33
@erdirowlands erdirowlands merged commit b9cb73e into main Oct 17, 2024
1 check passed
@erdirowlands erdirowlands deleted the FFM-12129 branch October 17, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants