-
Notifications
You must be signed in to change notification settings - Fork 33
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
chore: add js docs for context mutator hook #1045
Conversation
Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
* See the {@link https://openfeature.dev/docs/reference/technologies/client/web/#manage-evaluation-context-for-domains|documentation} for more information. | ||
* @default false | ||
*/ | ||
defaultContext?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wichopy @lukas-reining I've changed this from setGlobal
to defaultContext
for 2 reasons:
1 - I eventually want to return 2 functions from this hook, 1 for set
and one for merge
, so "set" seemed wrong
2 - we call this the default
context mostly in reference to other domain-scoped context
* @param updatedContext | ||
* @returns Promise for awaiting the context update | ||
*/ | ||
setContext: (updatedContext: EvaluationContext) => Promise<void>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wichopy @lukas-reining one thing perhaps worth talking about before we release is that we could make this return void
instead of Promise<void>
... it would give developers less power to "do something" after the context update is complete, but generally, developers should need to await this since things re-render if the context is changed.
IMO we should keep the promise exposed, but I wanted to point it out.
Signed-off-by: Todd Baert <[email protected]>
I'm going to merge this since it's clashing with some other work I'm doing, but please feel free to open new issues or make further comments before we release. |
🤖 I have created a release *beep* *boop* --- ## [0.4.7](react-sdk-v0.4.6...react-sdk-v0.4.7) (2024-10-29) ### ✨ New Features * avoid re-resolving flags unaffected by a change event ([#1024](#1024)) ([b8f9b4e](b8f9b4e)) * implement tracking as per spec ([#1020](#1020)) ([80f182e](80f182e)) * use mutate context hook ([#1031](#1031)) ([ec3d967](ec3d967)) ### 🧹 Chore * add js docs for context mutator hook ([#1045](#1045)) ([def3fe8](def3fe8)) * import type lint rule and fixes ([#1039](#1039)) ([01fcb93](01fcb93)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: OpenFeature Bot <[email protected]>
Minor refactors, but mostly adding js-doc after #1031.