Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Local Evaluation without awaiting a Promise #140

Closed
wants to merge 1 commit into from

Conversation

cody-greene
Copy link

@cody-greene cody-greene commented Dec 9, 2023

  • rm whitespace
  • rm superfluous updateEnvironment(); this was called 2x in local evaluation mode startup. Let the polling manager make the first call.
  • add getIdentityFlagsSync(); expects local evaluation or offline mode, and a synchronous cache. Does not return a Promise.
  • add readyCheck() Wait for the environment document to load. If the request fails then this promise is rejected. Useful for local evaluation mode.

I was rather confused that, with local evaluation mode, retrieving flags is still an async operation! Promises are entirely unnecessary in this particular situation. In fact, it forces any calling code to be async too which can be quite annoying. Once the environment document is first loaded, I should be able to check my flags in a simple synchronous fashion, while the environment updates in the background.

const flagsmith = new Flagsmith({
  enableLocalEvaluation: true
})

// need to wait for first load
await flagsmith.readyCheck()

// now evaluate with in-memory document
const flags =  flagsmith.getIdentityFlagsSync('unknown_user')
console.log(flags)

@cody-greene
Copy link
Author

Before I proceed any further with this (adding tests, better docs). Is this idea likely to be merged?

@matthewelwell
Copy link
Contributor

Before I proceed any further with this (adding tests, better docs). Is this idea likely to be merged?

Yes, this certainly seems like a good idea to me! We would love for you to continue, and we would definitely consider merging.

@dabeeeenster
Copy link
Contributor

@cody-greene Are you able to work on this? If not I'll close?

@cody-greene
Copy link
Author

cody-greene commented Mar 20, 2024 via email

@novakzaballa novakzaballa marked this pull request as draft April 24, 2024 16:07
@novakzaballa
Copy link
Contributor

I will convert this PR to a draft in the meantime. Please feel free to change it to Ready for Review whenever it gets completed.

@cody-greene cody-greene marked this pull request as ready for review April 26, 2024 15:30
@novakzaballa
Copy link
Contributor

@cody-greene I am closing this PR since it has not been active for a long time. Please feel free to re-open once you have a new version as per the above comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants