launchdarkly / js-sdk-common

Code shared between all LaunchDarkly client-side JS-based SDKs
Other
3 stars 26 forks source link

Expose a hook to surface when fresh flags have been fetched #69

Closed captain-jamesma closed 4 months ago

captain-jamesma commented 2 years ago

Problem

Our organization bootstraps our client using localstorage, so we get the "successful initialization" event before the requestor finishes fetching new flags.

We have some business-cases where we need to identify if our flags have been read from localstorage, or are fresh from the LD servers, but we have no hooks to determine if/when this has occurred.

Proposal

finishInit returns a promise that is swallowed, but it seems we could somewhat easily emit an event when that promise resolves to indicate we have fetched the most recent flags.

This could ultimately be exposed through a waitForFreshData function to be compatible with the existing API.

eli-darkly commented 2 years ago

I'm unclear on why the client.waitForInitialization() promise would not do what you want.

captain-jamesma commented 2 years ago

client.waitForInitialization() fires when flags are read from localstorage, not when they're returned from the server.

        // We're reading the flags from local storage. Signal that we're ready,
        // then update localStorage for the next page load. We won't signal changes or update
        // the in-memory flags unless you subscribe for changes
        flags = storedFlags;
        utils.onNextTick(signalSuccessfulInit);
        // ☝️this is where the initialization event gets fired (which is correct, the flags ARE ready to use)

        return requestor
          .fetchFlagSettings(ident.getUser(), hash)
          .then(requestedFlags => replaceAllFlags(requestedFlags))
          .catch(err => emitter.maybeReportError(err));
        // ☝️this is the promise I need to hook into to verify that my flags are fresh
      }

-- https://github.com/launchdarkly/js-sdk-common/blob/main/src/index.js#L661

eli-darkly commented 2 years ago

I see. That seems like a reasonable use case, but we'll have to think about the best way to represent it in the API.

Is there a particular reason you want this to be an event? Based on the statement "we need to identify if our flags have been read from localstorage", it sounded to me like it might be more about checking the current state of the SDK at a particular point in time— in which case a simpler method in the client API with a boolean return value might make more sense.

captain-jamesma commented 2 years ago

Apologies, I'll provide the concrete use-case -- there's almost certainly better API abstractions than what I initially proposed.

We're rolling out a new beta interface hosted on separate routes, so our main experience is hosted on / and our new experience is hosted on /beta/. When a user signs in, we want to ensure they're routed to the correct experience based on their inclusion in a launchdarkly segment. We also use launchdarkly for a slew of other feature flags, and leverage the built-in localstorage caching for improved fault-tolerance.

This is mostly working well, but we've ran into an edge-case: When a user has logged in previously and was not in the beta, launchdarkly caches their flag as false. If they are added to the beta while offline, and then attempt to login, the cached false value is evaluated before we get updated flags from launchdarkly's servers.

The proposed API would allow us to wait until the waitForFreshData promise resolves or rejects to evaluate our flags. If the promise resolves, we know we have the most recent data from the servers. If the promise rejects, we can just fall back to the flags in the localstorage cache.

is there a particular reason you want this to be an event? Based on the statement "we need to identify if our flags have been read from localstorage", it sounded to me like it might be more about checking the current state of the SDK at a particular point in time

If it's a cleaner API from the launchdarkly team's perspective to expose a status instead of an event, then we could certainly make that work by polling the status. We are effectively waiting for a particular promise to resolve, which maps more cleanly to an event on our end, but I completely understand that your APIs can't be completely custom tailored to our specific use-case.

kinyoklion commented 4 months ago

This was added as part of the inspector interfaces. https://docs.launchdarkly.com/sdk/features/inspectors/?q=inspec