Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(settings): Add nimbus experiments to AppContext #18524

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jonalmeida
Copy link
Contributor

Because

  • We want to fetch experiments that can target a user on the first render/launch.

This pull request

  • Adds uniqueUserId generation which would happen in Backbone into React as a prerequiste.
  • Adds a new call to experiments in the AppContext.

Issue that this pull request solves

Closes: FXA-9783

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

Other information (Optional)

  • In development, useMemo is called twice because of StrictMode. This will not happen in a production build.
  • Tests will be followed up in FXA-9784, but I'd happily take some links and pointers to equivalent features that I could base mine on top of.

- Adds uniqueUserId generation which would happen in Backbone into
  React as a prerequiste.
- Adds a new call to experiments in the AppContext.
region = region.toLowerCase();
}

initializeNimbus(uniqueUserId, { language, region } as NimbusContextT);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to get this into AppContext or some other kind of observable state so that I can write a very simple useExperiments which provides a consumer with the experiment info they need in that final form that they can use anywhere in the app.

I'm not sure what to do with the results from initializeNimbus here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, I should move this to AppContext.initializeAppContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is the right direction for what we want: the experiments are fetched before the first render.

I can see this happen very early on now:

Screenshot 2025-03-10 at 2 56 55 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latest patch now has my current path forward - it doesn't seem like the worst idea since we make gql fetches at the same place, it's going to be one more request that happens now.

* inlined code that was written using Backbone utilities which could not immediately be transferred over.
* @returns a new or existing UUIDv4 for this user.
*/
function getUniqueUserId(): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Placed this here for now, but I can move it to some other place.

@jonalmeida jonalmeida force-pushed the FXA-9783-new branch 2 times, most recently from 41919cd to cfeb790 Compare March 10, 2025 20:22
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant