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

Should after hooks be allowed to mutate the value? #48

Closed
Tracked by #42
justinabrahms opened this issue May 2, 2022 · 3 comments
Closed
Tracked by #42

Should after hooks be allowed to mutate the value? #48

justinabrahms opened this issue May 2, 2022 · 3 comments
Labels
documentation Improvements or additions to documentation

Comments

@justinabrahms
Copy link
Member

Pro: you can do a json -> instance mapping in there, which would be rad.
Con: subsequent hooks don't get the json value anymore, which may be very surprising to them.

@justinabrahms justinabrahms added the documentation Improvements or additions to documentation label May 2, 2022
@moredip
Copy link
Member

moredip commented May 4, 2022

I think another con would be it would make type signatures really fiddly, and/or we'd lose a lot of type safety.

@beeme1mr
Copy link
Member

beeme1mr commented May 6, 2022

We could allow the after hook to mutate the value but require that the type signature itself doesn't change.

@beeme1mr beeme1mr mentioned this issue May 24, 2022
5 tasks
@toddbaert
Copy link
Member

In addition to side-effects that don't impact the returned value at all (ending an OTel span, logging, etc), one of the use-cases we have for this is validation of a resolved flag value. However, that use-case is resolved simply by throwing in the after hook if the retrieved value is invalid, which results in the evaluation defaulting, which is probably exactly what we'd want.

That is to say, I think we can, for now, close this since both those basic use-cases are satisfied. I don't think we need to support mutation of the after value, for now. Please re-open if you disagree!

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

No branches or pull requests

4 participants