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

patch for issue 248 (send null instead of undefined) #250

Merged
merged 3 commits into from Mar 2, 2016

Conversation

@staticinstance
Copy link
Contributor

commented Feb 26, 2016

Fixes #248

staticinstance added some commits Feb 26, 2016

Fix for #248
undefined is not valid JSON, make the value null instead.

JSON.parse(undefined) will throw an exception while JSON.parse(null) is acceptable.

http://stackoverflow.com/a/14946821/5012948

@staticinstance staticinstance changed the title Staticinstance patch for issue 248 patch for issue 248 (send null instead of undefined) Feb 26, 2016

@staticinstance staticinstance force-pushed the staticinstance:staticinstance-patch-248 branch from 8f9c3a9 to 1c32e2a Feb 26, 2016

@nettofarah

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2016

Have you tried passing in an empty hash {} as opposed to null?
I'd be curious to see what would happen in that scenario. Would it be the same as sending an empty body?

I would also suggest squashing the three commits and maybe trying to write a commit message that explain why this check is in place.

@jbandi

This comment has been minimized.

Copy link

commented Mar 2, 2016

@nettofarah Passing in an empty hash {} has the same effect as passing null.

Only passing undefined has the undesired side-effect of IE changing the Content-Type header to text/plain;charset=UTF-8

Personally I find passing null more explicit/readable than passing {}. It's also what you find in most tutorials...

mzabriskie added a commit that referenced this pull request Mar 2, 2016

Merge pull request #250 from staticinstance/staticinstance-patch-248
patch for issue 248 (send null instead of undefined)

@mzabriskie mzabriskie merged commit a383bd5 into axios:master Mar 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mzabriskie

This comment has been minimized.

Copy link
Member

commented Mar 2, 2016

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.