adopted-ember-addons / ember-launch-darkly

A modern Ember addon to wrap the Launch Darkly service
MIT License
13 stars 24 forks source link

Support for LaunchDarkly's contexts upgrade #409

Closed tniezurawski closed 1 year ago

tniezurawski commented 1 year ago

LaunchDarkly plans changes in a way they track monthly usage. They will start the changes from the 1st of April and switch from MAU (Monthly Active Users) to MCI (Monthly Context Instances). You can read more about it here: https://docs.launchdarkly.com/guides/flags/upgrading-contexts.

To use contexts we would have to bump launchdarkly-js-client-sdk to version 3.0.0. I'm not yet sure what would be involved in the addon to start supporting it but just wanted to flag it

tniezurawski commented 1 year ago

It looks it should be as easy as setting an extra property to pass to identify() method:

let customProps = {
 someKey: 'some-value'
};
let props = {
  key: user.id,
  kind: 'my-new-context',  // "user" by default
  anonymous: false,
  custom: customProps,
};

await identify(props);

I think no changes, besides bumping launchdarkly-js-client-sdk to version 3.0.0, would be needed.


Feel free to close it, or close when we have the SDK upgraded 👌

SkoebaSteve commented 1 year ago

@tniezurawski thanks for sharing this. It's on my list to verify this at Qonto next week so if it's all ✅ we'll bump it after that.

SkoebaSteve commented 1 year ago

I validated it on our side and there's no breaking change with the addon as far as I can see so I merged it, it will be part of our new v3 release where we will drop some older ember/node versions.

I'll keep you posted, should be this week

tniezurawski commented 1 year ago

Great stuff @SkoebaSteve 🙌 I'm not super familiar with internals here and probably don't use everything that is possible in this addon but do you think this code:

https://github.com/adopted-ember-addons/ember-launch-darkly/blob/8c06fac44d184abfa9cf29850de7ec9b95e85cac/addon/-sdk/context.js#L112

might be affected by a breaking change and should be tweaked?

LDClient.getUser has been replaced with LDClient.getContext.

SkoebaSteve commented 1 year ago

@tniezurawski yep you're right, we actually were not relying on the user getter here ourselves but it breaks indeed:

image

I'll create a fix for this, thanks for spotting!