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

Update README.md with download as binary example #96

Merged
merged 5 commits into from
Aug 14, 2018

Conversation

muthurathinam
Copy link
Contributor

@muthurathinam muthurathinam commented Aug 9, 2018

README.md Outdated
````js
client
.api(`/me/drive/root/children/${fileName}/content`)
.responseType("blob")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should specify what the other magic strings options are for the responseType argument. Even better, we should have a ResponseType.ts enum with proper comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, its good to have a ResponseType.ts with comments..! Will update this pull request.

@muthurathinam
Copy link
Contributor Author

@MIchaelMainer

Updated with changes:

  1. Added separate ResponseType.ts file which has ResponseTypes declared as enum
  2. Updated the same in readme

Comments added for enum in ResponseType.ts will not be available in the corresponding .d.ts and .js files. As this compiler option modification comes in this change. We will recompile this .ts file to have comments in its .d.ts and .js files once we merge above change.

Yes, MicrosoftGraph.ResponseType.DOCUMENT does return a json response.
As mentioned in this pull request

Additional "Bug"
Setting the responseType to Document will result in JSON being returned.
Fetch API doesn't support this feature (whatwg/fetch#16) This return type doesn't really make sense for communicating with Microsoft Graph.

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

5 participants