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 if the body contains a File or FileList object will be converted to {} #183

Merged
merged 4 commits into from
Jan 23, 2019

Conversation

LuHugo
Copy link
Contributor

@LuHugo LuHugo commented Jan 15, 2019

Fix if the body contains a File or FileList object will be converted to {}

@apollo-cla
Copy link

@LuHugo: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@codecov-io
Copy link

codecov-io commented Jan 15, 2019

Codecov Report

Merging #183 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #183      +/-   ##
==========================================
+ Coverage   92.51%   92.55%   +0.03%     
==========================================
  Files           3        3              
  Lines         401      403       +2     
  Branches      116      117       +1     
==========================================
+ Hits          371      373       +2     
  Misses         28       28              
  Partials        2        2
Impacted Files Coverage Δ
src/restLink.ts 92.3% <100%> (+0.03%) ⬆️

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 4ee75b5...58f4398. Read the comment docs.

@fbartho
Copy link
Collaborator

fbartho commented Jan 15, 2019

If we can add a unit test for it, I’d be happy to ship it!

@LuHugo
Copy link
Contributor Author

LuHugo commented Jan 16, 2019

@fbartho In the current version, the bodySerializer function in bodySerializers can't get the File object, and the property in the body always returns {}, which is converted to a normal object.

I expect the body params

{
  name: "Alice",
  avatar: File
}

Actually I get the body params

{
  name: "Alice",
  avatar: {}
}

It was converted to a normal object, which prevented me from uploading files.

@fbartho
Copy link
Collaborator

fbartho commented Jan 16, 2019

I hear you @LuHugo. Is there a way to write a unit test that verifies that this behavior is working? I wouldn't want future regressions to break it accidentally.

@LuHugo
Copy link
Contributor Author

LuHugo commented Jan 17, 2019

@fbartho Unit test has been added

Copy link
Collaborator

@fbartho fbartho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woo! Thanks @LuHugo

@fbartho fbartho merged commit b071138 into apollographql:master Jan 23, 2019
@cloudever
Copy link

Release to npm?

@fbartho
Copy link
Collaborator

fbartho commented Mar 24, 2019

Published in v0.7.1:

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.

5 participants