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

Added async deserialize/serialize function types #349

Merged

Conversation

cesarfd
Copy link
Contributor

@cesarfd cesarfd commented May 27, 2022

Added Promised return types for serialize and deserialize.

@codecov
Copy link

codecov bot commented May 27, 2022

Codecov Report

Merging #349 (8d4705f) into main (8fc7f76) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #349   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1113      1113           
=========================================
  Hits          1113      1113           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fc7f76...8d4705f. Read the comment docs.

@jaredwray jaredwray merged commit e7449b5 into jaredwray:main May 30, 2022
@kaioduarte
Copy link

kaioduarte commented Jun 21, 2022

@jaredwray FYI, this change is breaking the Apollo server keyv adapter (v1.1.0)

The types returned by 'serialize(...)' are incompatible between these types.
    Type 'string | Promise<string>' is not assignable to type 'string'.
        Type 'Promise<string>' is not assignable to type 'string'.

@jaredwray
Copy link
Owner

@kaioduarte - thanks for bringing this to our attention. Do you know what the recommended fix is?

If not, I can dig in later this week to figure out what is the difference.

@kaioduarte
Copy link

@jaredwray, I'm not sure what would be the proper fix.

This issue is happening when you have keyv@^4.3.1 and @apollo/util.keyv-adapter@1.1.0 (which has keyv@^4.2.8 as a dependency, but keyv@4.2.8 it's in on the adapter package-lock.json).

I can create a PR to bump keyv version for the adapter, but it won't work if people have the adapter version fixed to 1.1.0.

@jaredwray
Copy link
Owner

@kaioduarte - we have a fix that should go out for 4.3.2 today we believe. It is now on main if you want to validate.

@kaioduarte
Copy link

@jaredwray amazing, thank you!

@jaredwray
Copy link
Owner

@kaioduarte - if you have any questions please create an issue so we can track it there.

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

Successfully merging this pull request may close these issues.

3 participants