Skip to content
This repository has been archived by the owner on Mar 16, 2019. It is now read-only.

Conflicting types (string, bool) for RNFetchBlobWriteStream.append property #461

Closed
lll000111 opened this issue Aug 6, 2017 · 0 comments

Comments

@lll000111
Copy link
Contributor

lll000111 commented Aug 6, 2017

I know you don't really use the Flow types (very unfortunate I must say), but this is a 10 second fix:

In file https://github.com/wkh237/react-native-fetch-blob/blob/master/class/RNFetchBlobWriteStream.js

there is append : bool;, but in the constructor there is append:string

I guess you could also change encoding to be the same as for the read stream class instead of just string:

encoding : 'utf8' | 'ascii' | 'base64';

PS: This class is not mentioned on Wiki page https://github.com/wkh237/react-native-fetch-blob/wiki/Classes, only RNFetchBlobReadStream EDIT: I added that class, didn't know I could actually do that as a random user :-)

QUESTION (about my edit on the Wiki page): It seems to me "detail" (link to source) is a string? I left out the type when I added that method because I'm not quite sure.


By the way, after completely ignoring encoding "ascii" for the longest time because I associate that with "old, only 7 bit per byte are used", I now found that the encoding called "ascii" actually means "Uint8Array", i.e. binary. I find the naming very unfortunate, to me it signals the complete opposite of "binary" (I don't think I'm alone in this?). Adding to the confusion is the existence of base64, so that I thought all binary data has to be base64 string encoded.

wkh237 pushed a commit that referenced this issue Aug 9, 2017
* bump to 0.10.8

* Update PULL_REQUEST_TEMPLATE

* Fix #468 "Messy error returns: Sometimes a string, sometimes an Error object"

* Cleanup: remove an unused constant and duplicate method definitions

* Cleanup:
- fix minor errors in JSDoc comments, for example {string]} => {string}
- fix parameter name "encode" => "encoding" (more logical, and it says so in the function's JSDoc too)
- json-stream.js: split a looooong log message string constant into two parts and fix a typo ("maually"), and the type for objects is "Object" (capitalized) in Flow type annotations

* Fix a (Flow) type conflict - fixes issue #461

* NEEDS REVIEW - Attempt to fix some of issue #460 (Error message normalization)

Error messages reported by iOS and Android versions should be as similar as possible. Also, within the same system there should be consistency. This patch is an attempt to bring a LITTLE more of this consistency to the error messages. I also fixed some very few minor language issues, like "does not exist" (is the correct English). I tried keeping the changes to a minimum.

Background: In my project code I want to know when a file already exists (e.g. after calling fs.createFile), and the only way is to check the error message string that I get. It's bad if they differ between versions (createFileASCII and createFile) and then also between Android and iOS version. At least some core part of the string should be the same, so that I have something to match.

Ideally messages should come from a centralized easy (easier) to maintain file (for both iOS and Android), and ideally both systems should have the same errors and messages as far as possible.
wkh237 pushed a commit that referenced this issue Aug 9, 2017
* bump to 0.10.8

* Update PULL_REQUEST_TEMPLATE

* Fix #468 "Messy error returns: Sometimes a string, sometimes an Error object"

* Cleanup: remove an unused constant and duplicate method definitions

* Cleanup:
- fix minor errors in JSDoc comments, for example {string]} => {string}
- fix parameter name "encode" => "encoding" (more logical, and it says so in the function's JSDoc too)
- json-stream.js: split a looooong log message string constant into two parts and fix a typo ("maually"), and the type for objects is "Object" (capitalized) in Flow type annotations

* Fix a (Flow) type conflict - fixes issue #461

* NEEDS REVIEW - Attempt to fix some of issue #460 (Error message normalization)

Error messages reported by iOS and Android versions should be as similar as possible. Also, within the same system there should be consistency. This patch is an attempt to bring a LITTLE more of this consistency to the error messages. I also fixed some very few minor language issues, like "does not exist" (is the correct English). I tried keeping the changes to a minimum.

Background: In my project code I want to know when a file already exists (e.g. after calling fs.createFile), and the only way is to check the error message string that I get. It's bad if they differ between versions (createFileASCII and createFile) and then also between Android and iOS version. At least some core part of the string should be the same, so that I have something to match.

Ideally messages should come from a centralized easy (easier) to maintain file (for both iOS and Android), and ideally both systems should have the same errors and messages as far as possible.

* Fixes #467 by improving the write() function of write streams: By resolving with the RNFetchBlobWriteStream instance instead of with "undefined" writes can now be chained:

RNFetchBlob.fs.writeStream(PATH_TO_FILE, 'utf8', true)
.then((ofstream) => ofstream.write('foo'))
.then((ofstream) => ofstream.write('bar'))
.then((ofstream) => ofstream.write('foobar'))
.then((ofstream) => ofstream.close())

Reference: #467 (comment)
danielsuo added a commit to danielsuo/react-native-fetch-blob that referenced this issue Feb 13, 2018
* upstream/0.10.9:
  Fixed problem with type casting (wkh237#513)
  My proposed 0.10.9 changes (wkh237#489)
  wkh237#268 Cancelled task should not trigger `then` promise function
  Add ability to cancel android DownloadManager fetches (wkh237#502)
  Fix iOS initialization race condition (wkh237#499)
  prevent UIApplication methods from being called on background thread (wkh237#486)
  Implemenet fs.hash() -- wkh237#439 "Feature: Calculate file hash" (wkh237#476)
  I forgot one error string for the Android readFile() method (wkh237#475)
  Fix for wkh237#467 (wkh237#472)
  Fix for issue wkh237#468, wkh237#461, wkh237#460 and minor cleanup (wkh237#469)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant