Skip to content

fix default case for responseType in WireClient._fetchRequest #1151 #1152

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

Merged
merged 2 commits into from
Jan 19, 2019
Merged

fix default case for responseType in WireClient._fetchRequest #1151 #1152

merged 2 commits into from
Jan 19, 2019

Conversation

iLiviu
Copy link
Member

@iLiviu iLiviu commented Jan 7, 2019

If no responseType property is present in options parameter to WireClient._fetchRequest,
the function defaults to document and throws an error. If responseType is undefined,
the function should treat the response as text. Also, if the response is treated as text,
the syntheticXhr.responseText property should be set to processedBody in order to not
break XHR compatibility.

If no responseType property is present in options parameter to WireClient._fetchRequest,
the function defaults to document and throws an error. If responseType is undefined,
the function should treat the response as text. Also, if the response is treated as text,
the syntheticXhr.responseText property should be set to processedBody in order to not
break XHR compatibility.
@raucao raucao requested review from DougReeder and a team January 8, 2019 08:39
@DougReeder
Copy link
Contributor

This code looks fine.

It would be nice to have the tests in dropbox-suite and googledrive-suite run for both XHR and fetch, as is done in wireclient-suite.

Copy link
Member

@silverbucket silverbucket left a comment

Choose a reason for hiding this comment

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

Would approve with tests

@iLiviu
Copy link
Member Author

iLiviu commented Jan 9, 2019

i will try and refactor the dropbox-suite and googledrive-suite and include tests for fetch when i get some time

refactored wireclient-suite, dropbox-suite and googledrive-suite to use common code for XHR and fetch() mocks, so now dropbox-suite and googledrive-suite will also test using fetch() beside XHR. 

fixed a test in dropbox-suite.

added a test for WireClient when responseType is not set
@raucao
Copy link
Member

raucao commented Jan 19, 2019

I can confirm that this fixes both Dropbox and GDrive when compared to 1.2.0. Code lgtm.

Good job, @iLiviu! 👏

@raucao raucao dismissed silverbucket’s stale review January 19, 2019 10:18

Tests have been added.

@raucao raucao merged commit e903dd9 into remotestorage:master Jan 19, 2019
@iLiviu iLiviu deleted the bugfix/1151-fetch_handle_text_responseType branch January 19, 2019 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants