-
Notifications
You must be signed in to change notification settings - Fork 39
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
Document context merging order for before hook ctx #125
Conversation
Signed-off-by: Justin Abrahms <[email protected]>
2f4e4c9
to
5527a48
Compare
Co-authored-by: Todd Baert <[email protected]> Signed-off-by: Justin Abrahms <[email protected]>
5527a48
to
f2ce332
Compare
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.
I dont love that this is a bit redundant with 3.2.2, but that was true before this change and I'm not sure the best way to resolve it. I think this is an improvement.
I think this made perfect sense before provider hooks, but now with them in the mix, things get more complicated. One of the primary use-cases of provider hooks is context transformation... if a provider hook does significant context transformation (say, into some vendor-specific object, or by serializing values in some required way) then it has to have the highest merge priority, right? So do we do: (top-most wins):
or (top-most wins):
|
Huh. I don't like our options now. :-P You're thinking that a context transformation use-case is going to set an additional property on the context that's specific for the provider? How about this (top-most wins)?
|
Well yes, but to be honest, I was thinking something even more potentially problematic - our context transformer allowed the context object to be an entirely new object/instance - for example an LDUser. I'm not sure that was understood 😅 . As you can imagine merging after that transformation is completely impossible. At the very least, I think provider hooks may transform the context in a way to maintain compatibility with their particular backend, so they need to "win". |
Thinking about it more, considering implementation difficulty and cognitive burden on application-authors, I think I'm in favor of: (top-most wins):
So hooks will always win... I think this is OK and simple to understand. If the author REALLY wants to override something in their invocation, they can use an invocation before hook to do that, which will override everything except the provider hooks (which is ideal). @justinabrahms @beeme1mr what do you guys think about that? |
I think I could be into that. So we're saying that:
|
Exactly. |
Signed-off-by: Justin Abrahms <[email protected]>
Signed-off-by: Justin Abrahms <[email protected]> Co-authored-by: Todd Baert <[email protected]> Signed-off-by: Justin Abrahms <[email protected]>
Fixes #124