Skip to content

Conversation

@Tymek
Copy link
Contributor

@Tymek Tymek commented Jun 13, 2024

There was a bug where the flagsReady status, returned from the useFlagsStatus hook, remains false even when an already started Unleash client is passed to the Unleash flag provider. This occurred despite the client being initialized and ready.

Closes #168

About the changes

  • test showing the issue
  • externalizing SDK state
    • Upgrade unleash-proxy-client dependency to version 3.5.1.
  • check state in Provider
    • Updated the initialization logic to check if the client is already ready and set flagsReady appropriately.

@Tymek Tymek marked this pull request as ready for review June 26, 2024 13:37
Boolean(
unleashClient
? customConfig?.bootstrap && customConfig?.bootstrapOverride !== false
? (customConfig?.bootstrap && customConfig?.bootstrapOverride !== false) || unleashClient.isReady?.()
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this the only significant line here? Why do we need these other changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's about adding the ability to pass initialized client. Error state is also needed to make sure state kept in React SDK is matching the one from initialized client

client.current.off('recovered', clearErrorCallback)
client.current.stop();
client.current.off('recovered', clearErrorCallback);
if (stopClient) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a stopClient property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you're passing an initialized client, you will probably also not want it to stop when React part of the app stops - in microfrontends for example

Copy link

@cemreyavuz cemreyavuz Jun 27, 2024

Choose a reason for hiding this comment

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

you will probably also not want it to stop when React part of the app stops - in microfrontends for example

(Sorry for jumping to the discussion) This is exactly what I was trying to achieve before creating the referenced issue - so I can say that this is an actual use case for sure ➕

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. Thank you for clarifying.

Copy link
Contributor

@FredrikOseberg FredrikOseberg left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Flags status is incorrect when an already started unleash client is passed to flag provider

3 participants