feat: client in memory provider

[luizgribeiro]

This PR

• Adds in memory provider implementation for client (web) sdk
• Move files from server sdk to shared

Related Issues

Closes #565

Notes

It's a first try. Any feedback and enhancement proposals are welcome :)

How to test

Automated/unit testes were implemented, but e2e tests wouldn't hurt.

[beeme1mr]

I didn't have a chance to give it a detailed review but I wanted to provide some quick feedback. Overall, it looks good but I had a few questions. Also, would it be possible to share some of the logic across client and server? @toddbaert is working on publishing the shared components right now. Perhaps that work could be leveraged here.

[lukas-reining]

This looks good to me!
Left some comments, mainly regarding the usage of the cache.
Please correct me if I see something wrong here.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a7d0b95) 99.07% compared to head (e3f6c87) 0.00%.

❗ Current head e3f6c87 differs from pull request most recent head d7536bc. Consider uploading reports for the commit d7536bc to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##             main   #617       +/-   ##
=========================================
- Coverage   99.07%      0   -99.08%     
=========================================
  Files          69      0       -69     
  Lines        2821      0     -2821     
  Branches      310      0      -310     
=========================================
- Hits         2795      0     -2795     
+ Misses         26      0       -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[luizgribeiro]

Hey folks! Thanks for the feedback.

Based on what you said, I'm planning on:

1. Getting rid of flag cache: I'd rather keep initialize and use it to guarantee that nothing breaks with context evaluation and set provider's status as ready
2. Unifying putConfiguration and putConfigurationAndUpdateResolution: I think @toddbaert said something about making the update and status cycle change invisible. It should probably call initialize internally then
3. Provide a way to change/control provider's status

What do you think about that? @beeme1mr @lukas-reining

[beeme1mr]

3. Provide a way to change/control provider's status

Is this for supporting various test use cases?

[lukas-reining]

Sound good to me @luizgribeiro :)

[toddbaert]

Hey folks! Thanks for the feedback.

Based on what you said, I'm planning on:

1. Getting rid of flag cache: I'd rather keep initialize and use it to guarantee that nothing breaks with context evaluation and set provider's status as ready

👍

2. Unifying putConfiguration and putConfigurationAndUpdateResolution: I think @toddbaert said something about making the update and status cycle change invisible. It should probably call initialize internally then

👍

3. Provide a way to change/control provider's status

I think this might add confusion. It's easy enough to make a testing provider to test this stuff, but because this is all "in-memory", I don't expect a lot of complexity around statuses. The only things I would expect in this care are NOT_READY (before init) READY (after succeessful init) and perhaps ERROR if somebody tries to put an invalid flag definition, but its not required for you to validate ahead of time - you could just throw during the eval if you'd like.

When implementing the in-memory provider, we should keep in mind 2 things:

• this is primarily a utility for testing flag evaluation "happy-paths", so that devs can experiment with all their code branches controlled by feature flags.
• it should conform to appendix B
• the implementation should be minimalist since people are likely to use it as a reference

[beeme1mr]

Hey @luizgribeiro, it looks like there are some merge conflicts since we moved some files to shared. Would you be able to update this PR to absorb those changes?

[luizgribeiro]

Hey @beeme1mr! Sure, I'll probably have some time to work on this during the weekend.

[luizgribeiro]

I've just pushed a newer version that instead of caching flag resolution it works based on the points mentioned above and store context internally to make it easier to test (even though it can also use context provided in resolution).
Let me know wyt.

Sorry for taking too long on this

[toddbaert]

Hey @luizgribeiro ! This latest work looks great. I did however revert all changes that weren't /client. My reasoning is threefold:

• this is a feat only for client, so we need to scope our changes there so the release automation only makes changes there
• artifacts specific to the in-memory provider, such as the VariantFoundError and the FlagConfiguration and Flag types cannot be exported from shared, or I think they will really confuse people. These are only implementation details for the in-memory provider, and not part of the actual SDK API. The only thing we should export is the in-memory provider itself.
• in addition to being useful in tests, this serves as an example, so a bit of duplication between client/server isn't bad in this case, IMO

I made only a few actual changes to the provider itself, which I've noted here.

[toddbaert]

It may be worth adding this as a comment to the provider since people may use the in-memory provider as a point of reference.

@beeme1mr good idea. I will do this generally.

[toddbaert]

Thanks again @luizgribeiro . I will merge this if you don't have any objections to my changes to your branch.

[luizgribeiro]

That looks good @toddbaert! Fell free to merge it 🚀