launchdarkly / react-client-sdk

LaunchDarkly Client-side SDK for React.js
Other
81 stars 67 forks source link

LD doesn't start if user provided later #130

Open janko-sokolovic opened 2 years ago

janko-sokolovic commented 2 years ago

Is this a support request? This issue tracker is maintained by LaunchDarkly SDK developers and is intended for feedback on the SDK code. If you're not sure whether the problem you are having is specifically related to the SDK, or to the LaunchDarkly service overall, it may be more appropriate to contact the LaunchDarkly support team; they can help to investigate the problem and will consult the SDK team if necessary. You can submit a support request by going here or by emailing support@launchdarkly.com.

Note that issues filed on this issue tracker are publicly accessible. Do not provide any private account information on your issues. If your problem is specific to your account, you should submit a support request as described above.

Describe the bug When initializing the LD by wrapping the App component, BUT without providing user (because user is known only after login), and then later initializing user via LDClient, no requests are sent and no flags received...

To reproduce Initialize LD via withLDProvider({clientSideId: 'asdasdasd'})(App) but provide user at later stage (after login, one deeper component), via

const ldClient = useLDClient();
useEffect(() => {
    ldClient?.identify({
      "key": "somekey",
      "name": "somename",
      "email": "name@company.com"
    } )
  }, [ldClient]);

this will not trigger flags to be loaded or are undefined

Expected behavior Flags are loaded similarly to how it works when user is provided in initial config

Logs n/a

SDK version 2.25.1

Language version, developer tools JS/TS/React

OS/platform Chromium-based browser

Additional context Add any other context about the problem here.

louis-launchdarkly commented 2 years ago

Hello @janko-sokolovic, thank you for reaching out. Based on the documentation here, you will need to pass in a dummy user object to initialize the LDClient.

If you don't have a user at the moment of initialization, you can follow the instruction here to use a single shared key.

If you need to track the anonymous user for analytics and insights, you can follow the instruction here.

allicanseenow commented 2 years ago

It might be somewhat related but right now if ldClient?.identify is called, there is no way to observe whether the user object has changed, unless I make my own context/HOC/redux reducer to trigger other components to rerender. As far as I'm concerned, the only way to fetch the user data is through ldClient?.getUser(), but this function won't be called again automatically without the component's rerendering as the object ldClient is stable.

It would be nice if we can actually observe the user variable, or at least the useFlags() hook can provide this extra info.

zachgoll commented 2 years ago

@louis-launchdarkly while I understand that using an anonymous user for initialization "fixes" this, I wanted to provide a specific case where it does not appear to behave correctly.

In this example, let's say that I initialized with the user, { key: 'anonymous-client', anonymous: true }, my flag that I'm using is shouldShowNewModal, and I have a test user called { key: 'engineering-ci-user' }.

In our use case, we want shouldShowNewModal to evaluate to true in ALL cases EXCEPT for our engineering-ci-user. We're doing this with the following config:

image

The problem here is that because we're initializing with a static, anonymous user (that has no utility other than preventing quota overages and initializing the client), we get 2 flag evaluations on each page load:

  1. { shouldShowNewModal: true } - first evaluation is against anonymous-client, which returns true
  2. { shouldShowNewModal: false - second evaluation is against engineering-ci, which returns false

In most cases, this isn't a problem, but on slow networks (or while running e2e tests), it causes bugs. Our workaround is twofold:

  1. We set { key: 'anonymous-client', anonymous: true } at initialization
  2. We specifically exclude this user in targeting, which is not obvious as this user does not appear as an option in the users dropdown, and even highlights a warning color (see below)

image

Ideally, we'd be able to set deferInitialization: true, NOT set the anonymous user, and set our logged-in user when ready with ld.identify({ ... })

XieX commented 1 year ago

@allicanseenow

right now if ldClient?.identify is called, there is no way to observe whether the user object has changed

You're right, though LDClient#identify returns a promise that resolves (or you can provide a callback that is called) once it's finished loading the new user's flags - while not exactly what you're asking for, you could use that in a custom hook to know when you can hit getUser. Something like,

export default function useLDUser() {
  const ldClient = useLDClient()
  const [user, setUser] = useState()

  useEffect(() => {
    if (!ldClient) return;
    setUser(ldclient.getUser())
  }, [ldClient])

  const identify = useCallback(
    (newUser) => ldClient && ldClient.identify(newUser)
      .then(() => setUser(ldClient.getUser())), [ldClient])

  return [user, identify]
}

This is off the top of my head, so it may not work as-is... :p But you get the idea.

cejaramillof commented 1 year ago

Hello @janko-sokolovic, thank you for reaching out. Based on the documentation here, you will need to pass in a dummy user object to initialize the LDClient.

If you don't have a user at the moment of initialization, you can follow the instruction here to use a single shared key.

If you need to track the anonymous user for analytics and insights, you can follow the instruction here.

Hi @louis-launchdarkly , I have the same doubt, I understand that sending some base data could solve it, but if I do this I am not clear about the functionality of the deferInitialization in true because with the mock data it would initialize as if it was false, right? Could you help me? thanks

tlehwalder commented 1 year ago

are there any updates on this? I also try to use withLDProvider and deferInitialization: true

The documentation says:

deferInitialization: This property allows you to defer SDK initialization until you define the context property. This property is optional, and is only available in ProviderConfig, when you initialize using withLDProvider.

but does not say how to define context afterwards.

using ldClient?.identify(...) (with ldClient from useLDCLient hook) is not possible, since it is undefiend

olexpono commented 1 year ago

+1 to @tlehwalder 's explanation of the core issue, having the same challenge.

The help docs have an article on how to pass a deferred context into LDProvider, but the 3.0.X docs so far only explain how to initialize using the HOC withLDProvider and the async version.

Because the HOC takes a component type and not an instance of a component / children prop, it becomes difficult to get a context later and re-render it in a component tree, like LDProvider is in the help article.

Like @janko-sokolovic, I expected the client to initialize and load flags when deferInitialization: true and a new context is set using identify(). I'm able to get the effect I want by running deferInitialization: false, context: {} and ignoring the 400 error, then calling identify, but obviously that's not ideal for either party.

In our use-case, we have a service dishing out anonymous user IDs (a-la Segment anonymous ID), and we don't want flag values to be evaluated before we have a known anonymous ID for this session.

It would be nice to get a practical example of how deferInitialization can be used with withLDProvider, and this should probably be linked as a 3.0.x addendum in the help article linked above.

louis-launchdarkly commented 11 months ago

Hello all, sorry for the late reply. I am reaching out to the React engineers on the team to double check is the deferInitialization can work with withLDProvider, if so, we will look for a way to provide examples on how this can work.

Filed Internally as 212132.

lsanwick commented 9 months ago

@louis-launchdarkly Any update on whether this is possible? I'm trying to rewrite our provider to use deferInitialization and withLDProvider and I'm running into React Maximum update depth exceeded errors in some situations.

olexpono commented 9 months ago

The LD team reached out (via support channel) and created this example a couple months ago. This uses the LDProvider export directly and gives an example of deferInitialization, without the HOC's:

https://github.com/launchdarkly/react-client-sdk/blob/51748c30e22147f485653d9c10a7eac7603b1606/examples/typescript/src/App.tsx

I was able to solve my deferInitialization troubles and get an after-first-render LD context firing without too much trouble starting with this.

@lsanwick Check out this example and see if it works for what you're building!

fergy-os commented 9 months ago

@olexpono that code example makes sense but how does the prevent the issue described here from happening?

luka-bacic commented 8 months ago

Hello @zachgoll,

At work I was facing this exact same issue and a similar use-case you are describing. For our specific use-case, having flags initialized as anonymous users was completely irrelevant and useless, because we default flag values to false, and use Segment targeting based on user attributes to set them to true. As you can imagine, initial render had flags evaluate as false, then a few renders later they become true.

High level of how we got it working well for our usecase:

  1. We don't use withLDProvider nor asyncWithLdProvider anymore
  2. We directly use LDProvider from launchdarkly-react-client-sdk
  3. We added our own custom feature flags context below LDProvider which exposes the feature flags and the ldClient
  4. We wrap the ldClient in a custom class, which exposes ldClient.identify, and accepts an onSuccess callback to allow us to force a rerender once we initialise the user
  5. We show loaders until launch darkly initializes a non-anonymous user
  6. We added 2 custom hooks to get ldClient and feature flags from our custom context from step 3

Here is a stripped down version of how we made it work (had to remove lots of sensitive lines, and some of these are pseudocode, adjust based on your app). I believe this should be enough to give you the general idea of our approach.

The code for the custom provider:

import LaunchDarkly from "./LaunchDarkly"; // our custom wrapper class
import Loader from "./Loader";
import {useFlags, LDProvider, useLDClient, ProviderConfig } from "launchdarkly-react-client-sdk";
import React, { createContext, useContext, useState } from "react";
import { LDFlagSet } from "launchdarkly-js-sdk-common";

const FeatureFlagsContext = createContext<
    | {
          flags: LDFlagSet;
          ldClient: LaunchDarkly;
      }
    | undefined
>(undefined);

// you add this provider in your App.tsx file
export function FeatureFlagsProvider(props: {
    children: React.ReactNode;
    providerProps?: Omit<ProviderConfig, "context">;
}) {
    return (
        <LDProvider {...props.providerProps} context={{ kind: "user", anonymous: true }}>
            <FeatureFlagsChildren children={props.children} />
        </LDProvider>
    );
}
function FeatureFlagsChildren(props: { children: React.ReactNode }) {
    const ldClientRaw = useLDClient();
    const flags = useFlags();
    const [, forceRerender] = useState(Date.now());

    // first level loader - doesn't allow to render anything below the provider unless ldClientRaw get initialised with the anonymous user
    if (!ldClientRaw) return <Loader />;

    // this is our custom built class, it is NOT exported by launch darkly. The gist of it is that
    // it wraps the ldClient, exposes its `identify` method, and we force a rerender once identify
    // resolves (see next code snippet below for this class)
    const ldClient = new LaunchDarkly(ldClientRaw, () => forceRerender(Date.now()));

    return (
        <FeatureFlagsContext.Provider value={{ flags, ldClient }}>
            {props.children}
        </FeatureFlagsContext.Provider>
    );
}

// use this hook in your code instead of the official launch darkly exported hook `useLDClient` if
// you need to access the client. this returns the custom class instance which wraps ldClient
export function useLaunchDarklyClient() {
    const data = useContext(FeatureFlagsContext);
    if (!data) throw new Error("Launch Darkly client has not initialized yet");
    return data.ldClient;
}

// use this hook to access flags. it handles when the client isn't initialised, and when the flags
// haven't returned yet
export function useSafeFeatureFlags() {
    const data = useContext(FeatureFlagsContext);
    if (!data) throw new Error("Launch Darkly client has not initialized yet");
    if (!data.flags) throw new Error("Flags haven't initialised yet");
    return data.flags;
}

the custom class which wraps ldClient:

import type {LDClient} from 'launchdarkly-js-client-sdk';

export default class LaunchDarkly  {
    private _client: LDClient;
    private onSuccess: () => void;

    constructor(lDClient: LDClient, onSuccess: () => void) {
        this._client = lDClient;
        this.onSuccess = onSuccess;
    }

    isAnonymousContext() {
        return this._client.getContext()?.anonymous === true;
    }

    identify(user: Record<string, any>) {
        this._client
            .identify({
                kind: 'user',
                key: user.id
                // ...other user attributes you care about
            })
            .then(() => this.onSuccess()); // this will cause the rerender if you pass a setState call in it
    }
}

and then finally in your App.tsx:

import { FeatureFlagsProvider,  useLaunchDarklyClient} from "./FeatureFlagsContext";

export default function App() {
    return (
        <FeatureFlagsProvider>
            <FetchUserData />
        </FeatureFlagsProvider>
    );
}

function FetchUserData() {
    const { isPending, data } = useFetchUserData();
    if (isPending) return <Loader />;
    return <WithFeatureFlags user={data.user} />;
}

function WithFeatureFlags(props: { user: User }) {
    const ldClient = useLaunchDarklyClient();
    useEffect(() => {
        ldClient.identify({
            id: props.user.id,
            // pass in other user attributes you care about
        });
    }, []);

    // the second level loader - important to add as well. this loader will stop showing once the
    // `identify` call above resolves
    if (ldClient.isAnonymousContext()) return <Loader />;

    // now anything that renders after this point will have the correct flags, and will not cause a flicker. 
    // Be sure to use the custom hook `useSafeFeatureFlags`, and not `useFlags` which is exported from `launchdarkly-react-client-sdk`
    return <ActualApp />;
}

and just for clarity, this is the index.tsx file (where you mount React). notice it does not have withLDProvider or asyncWithLdProvider in it

import React from 'react';
import {createRoot} from 'react-dom/client';
import App from './App';

createRoot(document.getElementById('root')!).render(<App />);

module.hot?.accept();
kenny-wonderschool commented 7 months ago

My team encountered the same issue. I'm frankly surprised a service of LD's reputation has such a critical issue ongoing for such a long time and that the docs recommend broken behavior:

image https://docs.launchdarkly.com/sdk/client-side/react/react-web#initializing-using-asyncwithldprovider

For a simple fix,

const LDProvider = await asyncWithLDProvider({ clientSideID: 'client-side-id-123abc' });

should be changed to

const anonContext = { kind: 'user', key: 'anonymous-id', anonymous: true };
const LDProvider = await asyncWithLDProvider({ clientSideID: 'client-side-id-123abc', context: anonContext });

I had enjoyed using LD at a previous company and and recommended it for my current team. This kind of issue really eroded a lot of trust I had built up.

Please tell me there's some kind of resolution planned so I can convince other teams this is still the right solution that we should be paying for.