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

fix(typescript): ssm context is enriched correctly #1089

Closed
wants to merge 4 commits into from

Conversation

lmammino
Copy link
Member

@lmammino lmammino commented Aug 25, 2023

Fixes #1084

@m-radzikowski, I have a first draft for a potential solution to what you suggested in #1084. I am not sure I understood your point about JsonValue.

I'd love if you can give this a quick review and let me know if this is going in the right direction.

On a second look at what I originally did in this PR, I understood that what I did was only testing the current existing solution for ssm. 😓

Now I think I understand that your suggestion is to generalise that approach to something that would work for all the other middleware that change the context. Is that correct?

With that being said, there's a bit of TS magic here that I am not entirely comfortable with... Can you, please, expand a bit on your pseudo-code maybe providing something more concrete that I can work with?

Thanks

@m-radzikowski
Copy link
Contributor

See my WIP PR with basic implementation: #1093

@lmammino
Copy link
Member Author

Closing in favour of #1093

@lmammino lmammino closed this Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants