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.

Sign up for GitHub

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

Jump to bottom

[BUG] typings don't support returning Promise<void> #689

Closed
toddbaert opened this issue Nov 27, 2023 · 3 comments · Fixed by #693
Closed

[BUG] typings don't support returning Promise<void> #689

toddbaert opened this issue Nov 27, 2023 · 3 comments · Fixed by #693
Assignees
@beeme1mr
Labels
bug Something isn't working

Comments

@toddbaert
Copy link
Member

Observed behavior

It looks like we have an error with the typings for the hooks when it comes to returning a Promise<void> from the server-sdk:

image

'Promise' is not assignable to type 'Promise' is the error.

I think the problem is here. We have Promise<EvaluationContext | Promise<void>>. I think this should read: Promise<EvaluationContext> | Promise<void> instead, or I'm missing something.

cc @beeme1mr @lukas-reining

Expected Behavior

No response

Steps to reproduce

No response
@toddbaert toddbaert added the bug Something isn't working label Nov 27, 2023
@beeme1mr
Copy link
Member

Yes, It looks like the current type is expecting a nested promise.

@beeme1mr
Copy link
Member

The behavior prior to this change was Promise<EvaluationContext | void> | EvaluationContext | void;.

@lukas-reining
Copy link
Member

lukas-reining commented Nov 27, 2023 •
edited
Loading

Ah that's a typo 🤦‍♂️
Yes, clearly it should not be nested.

@beeme1mr beeme1mr self-assigned this Nov 27, 2023
@beeme1mr beeme1mr mentioned this issue Nov 27, 2023
Merged
@beeme1mr beeme1mr linked a pull request Nov 27, 2023 that will close this issue
fix: type support for Promise<void> before hook #693
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 30, 2023
@beeme1mr @toddbaert
fix: type support for Promise<void> before hook (#693)
0b9ca18 
## This PR

- Updates the server-side before hook type to perform async work without
modifying context.

### Related Issues

Fixes #689

### Notes

I added a test but the test runner isn't not validating types.

---------

Signed-off-by: Michael Beemer <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@beeme1mr beeme1mr

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: type support for Promise<void> before hook open-feature/js-sdk
3 participants
@beeme1mr @toddbaert @lukas-reining