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

Feat various cleanup #65

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Feat various cleanup #65

wants to merge 5 commits into from

Conversation

urkle
Copy link
Contributor

@urkle urkle commented Aug 20, 2022

This performs a few small pieces of cleanup and fixes.

This PR include the changes from
#22
#49
#55

And obviates #64 (update of packages no longer includes this minimist)

Also now that there is jest test rig this PR ensures tests were added for all the above (or tests passing in the case of #49)

@@ -191,8 +203,7 @@
* {a: {b: {c: 1, d: 2}}}, "a.b.c" => 1
*/
GraphQLClient.prototype.fragmentPath = function (fragments, path) {
var getter = new Function("fragments", "return fragments." + path.replace(/\./g, FRAGMENT_SEPERATOR))
var obj = getter(fragments)
var obj = fragments[path.replace(/\./g, FRAGMENT_SEPERATOR)]
Copy link
Owner

Choose a reason for hiding this comment

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

why this is changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's equivalent, safer, and mentioned in #49

@urkle
Copy link
Contributor Author

urkle commented Aug 21, 2022

@f I removed the "content-length" commit from this PR, as apparently that is a forbidden header by the XMLHttpRequest, so there is no point in manually setting it, the browser should be doing that work for us.

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.

None yet

3 participants