startClient=false doesn't work if unleashClient prop is unspecified

[mltsy]

Describe the bug

When mounting the FlagProvider by passing in a config, rather than an unleashClient, I can't prevent the client from starting using startClient={false}. This should work, as is described in the v3 > v4 upgrade docs in the Readme as updated added here

Steps to reproduce the bug

Mount a FlagProvider that looks like this:

const config = {
  url: 'http://loclahost:9999',
  clientKey: 'anything'
  refreshInterval: 15,
  bootstrap: []
  }
//  ...
<FlagProvider config={config} startClient={false}>
  <Component />
</FlagProvider>

Then open the page in a browser.

Expected behavior

It should not poll http://localhost:9999 every 15 seconds (see network tab)

Logs, error output, etc.

No response

Screenshots

Happy to add a screenshot of my dev tools network tab, but... it just shows a long list of requests to the proxy URL ;)

Additional context

Here's the line causing the problem: https://github.com/Unleash/proxy-client-react/blob/main/src/FlagProvider.tsx#L81

In my case, the thing we're trying to do is just prevent starting the client in dev environments, based on an environment variable. I see in the code that there's also a config option called disableRefresh, which would be another potential solution, but currently the documentation (for this package) indicates that startClient={false} should work and doesn't mention disableRefresh.

Unleash version

5.6.6

Subscription type

Open source

Hosting type

Self-hosted

SDK information (language and version)

unleash-client-nextjs

[FredrikOseberg]

@mltsy Thanks for raising this!

I did some digging here and I think this is actually working as intended. The startClient option only works if you are passing in an unleashClient instead of directly passing config. This is because there is no way to have direct control over the client and start it at a later point if you pass in the config, since the config will be used to initialise the client inside of the FlagProvider.

If you instead new up the client with a config and inject it into the FlagProvider, the startClient option is respected:

import ReactDOM from 'react-dom/client';
import { FlagProvider } from '@unleash/proxy-client-react';
import { UnleashClient } from '@unleash/proxy-client-react';
import './index.css';
import App from './App';


const client = new UnleashClient({ url: 'unleashURL/api/frontend',
  clientKey:
    'KEY',
  appName: 'React app',
  refreshInterval: 2
  })

const root = ReactDOM.createRoot(
  document.getElementById('root') as HTMLElement
);

root.render(
  <FlagProvider unleashClient={client} startClient={false}>
    <App />
  </FlagProvider>
);

If you don't want to do this, another way to achieve what you want is to set disableRefresh and disableMetrics. This client is a wrapper around the main library: https://github.com/Unleash/unleash-proxy-client-js and will respect any config settings listed here.

Hopefully this clears up the confusion. I'll make a note of updating the documentation for this library to make it clear!

[mltsy]

Thanks!

Okay, yeah this makes sense to me, that for my use case (disabling API calls in a dev environment), the disableRefresh and disableMetrics options are more appropriate. In that case, yes, updating the docs would be great! The part that made me think this was a bug was in the "Migration Guide" section of the README where it says specifically:

startClient option has been simpilfied. Now it will also work if you don't pass custom client with it

which seems to just be incorrect?

But then yeah, it'd be awesome to either specifically call out dev-environment options like disableRefresh in an additional section under Advanced Use Cases, or even just mention that "the config prop of FlagProvider accepts all options documented for new UnleashClient in the unleash-proxy-client-js docs"

[FredrikOseberg]

Thanks!

Okay, yeah this makes sense to me, that for my use case (disabling API calls in a dev environment), the disableRefresh and disableMetrics options are more appropriate. In that case, yes, updating the docs would be great! The part that made me think this was a bug was in the "Migration Guide" section of the README where it says specifically:

startClient option has been simpilfied. Now it will also work if you don't pass custom client with it

which seems to just be incorrect?

But then yeah, it'd be awesome to either specifically call out dev-environment options like disableRefresh in an additional section under Advanced Use Cases, or even just mention that "the config prop of FlagProvider accepts all options documented for new UnleashClient in the unleash-proxy-client-js docs"

After considering this some more, I think it makes sense for the startClient option to also work if you pass in a config. I think we can classify this as a bug, since our own readme does claim that it is how it's supposed to work. I'm in the process of making a PR that will fix the issue.

[FredrikOseberg]

@mltsy I've released 4.2.2-beta.0 which should take care of this issue. Would you mind giving it a spin before I make it offical?

[mltsy]

I'll try to give it a shot today. (I'm actually using @unleash/next in my app, not this package directly, which might post a challenge, but I'm hoping I can figure out how to specify this package's version independently...)

[mltsy]

Bingo! 🎯 Works when it's true, works when it's false 😄 Thanks a lot @FredrikOseberg! 🙇

[FredrikOseberg]

Bingo! 🎯 Works when it's true, works when it's false 😄 Thanks a lot @FredrikOseberg! 🙇

Thank you much for the report! I'll release 4.2.2 shortly.

[mltsy]

Thanks again! I've opened a PR to unleash-client-nextjs to import this new version there... not sure if you have commit rights there, or if there's someone I should ping about that? Unleash/unleash-client-nextjs#67

(I haven't seen any response to my other PR in that repo from last week)