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

response.buffer() and blob fields doesn't exist in ReactNative #7862

Merged
merged 1 commit into from Aug 7, 2019

Conversation

garfieldnate
Copy link
Contributor

Add a check for ReactNative next to the checks for node/browsers so that we don't throw errors using unsupported API's.

Fixes #7688.

@garfieldnate
Copy link
Contributor Author

If this needs to be cleaned up or moved into a method or something, please let me know and I'll fix the PR up a bit. For now I would like to know whether the logic change is acceptable or not.

There are probably other environments where this is applicable: Ionic, VueNative, etc. I really just want support for ReactNative :)

@daleharvey
Copy link
Member

If we could switch these to feature detecting I think that would be ideal, they probably should have been already but definitely if we are going to handle more environments then definitely better than checking for environment names, something like:

if ('buffer' in response) { 

?

garfieldnate added a commit to garfieldnate/flashcards that referenced this pull request Aug 2, 2019
What an adventure today was! I spent the whole day, minus lunch and a short
meeting with Rupesh, debugging this. If you want to know how long that was,
check the commit timestamp :) I did learn how to debug from VS code, which is
cool, although apparently I may have to restart my computer once in a while T_T

Here are my notes from tracking the problem down:

- going back to previous commits and redoing some work
- resetting DB
- resetting device
- pushing data from app and trying to reload it (pushing only works sometimes)
- checking that kurakura is not running (shouldn't matter, but whatever)
- Updating dependencies (including Expo and even node!)
- pouchdb-adapter-http's fetchJSON keeps returning this stuff: {"\_40":0,"\_65":0,"\_55":null,"\_72":null} (oh wait, that's what a Promise looks like for some reason!). Actually fetchJSON returns stuff correctly. goes to http://localhost:5984/flashcards/_changes?style=all_docs&since=0&limit=100, gets back big thing with all flashcards.
- made pouchdb adapters same version as RxDB's pouchdb core dependency
- changing order of RxDB.plugin calls
- had to downgrade XDL for debugging to work in VSCode
- debugging (at least with VS code plugin) is difficult; expo app freezes when I try to restart debugging
- do `yarn add pouchdb-debug` and then find "comment in to debug" in RxDB/pouch-db.js. Did nothing.
- Copied that section into my own code and now I can see PouchDB debug statements!

    const pouchdbDebug = require('pouchdb-debug');
    RxDB.plugin(pouchdbDebug);
    RxDB.PouchDB.debug.enable('*');

- Apparently revs can be missing in change sequences, so patch RxDB/utils.js:

    export function getHeightOfRevision(revString) {
      if (!revString) {
        return 0;
      }
      const first = revString.split('-')[0];
      return parseInt(first);
    }

- Set process.browser = true globally to make sure pouchdb-adapter-http uses response.blob() instead of .buffer()
- Sent two PR's regarding these issues
- Getting  "Creating blobs from 'ArrayBuffer' and 'ArrayBufferView' are not supported", but can't get the call stack. Annoying pouchdb reports this as not found instead of error :( The attachments are definitely being found.
- Tracked message to react-native/Libraries/Blob/BlobManager.js in the Blob constructor.
- calling PouchDB's getAttachment() with options = { binary: false } gets ignored because PouchDB tries to do the most "efficient" thing on its own. Gets set to "true" in node_modules/pouchdb-core/lib/index.js before it gets to pouchdb-adapter-asyncstorage/src/get_attachment.js, where it wreaks havoc. Have to patch it to `false` in that file or... find out how to make it always false in PouchDB somewhere.
- Can't patch it in PouchDB because the docs explicitly say that `getAttachment()` always returns a blob. I don't think they will be happy being asked to change that, because then all of their plugins would have to support non-binary queries :( But now the plugins all have to assume that the `binary` option actually means "binary if possible otherwise not". adapter-asyncstorage actually uses this option, but it can't have ever worked! The docs even say that it is supposed to be for RN, so why...?
- URL.createObjectURL(blob) doesn't work in RN; changed to `'data:image/png;base64,' + blob`
- `FileReader.readAsArrayBuffer` polyfill is still required for the HTTP attachment pulling

***

Quick overview of the fixes we have:

PR's sent:

1. RxDB doesn't handle missing revision IDs: pubkey/rxdb#1347
2. PouchDB's feature detection logic not working for HN: pouchdb/pouchdb#7862

Local patches:

1. Fix for RxDB not handling missing revision IDs
2. Fix for pouchdb-adapter-asyncstorage trying to respect `options.binary`,
(PouchDB always sets to `true`, must be `false` to prevent errors in RN).

The PouchDB feature detection logic is also fixed by simply setting
`process.browser = true;` for now, since it was even easier than a patch and
should be temporary.

Finally, we also keep the polyfill for `FileReader.prototype.readAsArrayBuffer`,
which is needed for reading the binary attachment data from the server.

***

So at the end of the day, I have two GH tickets and two local patches. It may be better to open a third regarding the hardcoding of `options.binary` to `true` in PouchDB.core's `getAttachments()`, but I wouldn't now how to solve it. I just know it's problematic. In the longer wrong I would like both PR's accepted, and both patches removed (one fixed by the PR, the other by switching to an RN sqlite adapter).
garfieldnate added a commit to garfieldnate/flashcards that referenced this pull request Aug 2, 2019
What an adventure today was! I spent the whole day, minus lunch and a short
meeting with Rupesh, debugging this. If you want to know how long that was,
check the commit timestamp :) I did learn how to debug from VS code, which is
cool, although apparently I may have to restart my computer once in a while T_T

Here's a chronological list of everything I tried and some notes about what I
found:

- going back to previous commits and redoing some work
- resetting DB
- resetting device
- pushing data from app and trying to reload it (pushing only works sometimes)
- checking that kurakura is not running (shouldn't matter, but whatever)
- Updating dependencies (including Expo and even node!)
- pouchdb-adapter-http's fetchJSON keeps returning this stuff: {"\_40":0,"\_65":0,"\_55":null,"\_72":null} (oh wait, that's what a Promise looks like for some reason!). Actually fetchJSON returns stuff correctly. goes to http://localhost:5984/flashcards/_changes?style=all_docs&since=0&limit=100, gets back big thing with all flashcards.
- made pouchdb adapters same version as RxDB's pouchdb core dependency
- changing order of RxDB.plugin calls
- had to downgrade XDL for debugging to work in VSCode
- debugging (at least with VS code plugin) is difficult; expo app freezes when I try to restart debugging
- do `yarn add pouchdb-debug` and then find "comment in to debug" in RxDB/pouch-db.js. Did nothing.
- Copied that section into my own code and now I can see PouchDB debug statements!

    const pouchdbDebug = require('pouchdb-debug');
    RxDB.plugin(pouchdbDebug);
    RxDB.PouchDB.debug.enable('*');

- Apparently revs can be missing in change sequences, so patch RxDB/utils.js:

    export function getHeightOfRevision(revString) {
      if (!revString) {
        return 0;
      }
      const first = revString.split('-')[0];
      return parseInt(first);
    }

- Set process.browser = true globally to make sure pouchdb-adapter-http uses response.blob() instead of .buffer()
- Sent two PR's regarding these issues
- Getting  "Creating blobs from 'ArrayBuffer' and 'ArrayBufferView' are not supported", but can't get the call stack. Annoying pouchdb reports this as not found instead of error :( The attachments are definitely being found.
- Tracked message to react-native/Libraries/Blob/BlobManager.js in the Blob constructor.
- calling PouchDB's getAttachment() with options = { binary: false } gets ignored because PouchDB tries to do the most "efficient" thing on its own. Gets set to "true" in node_modules/pouchdb-core/lib/index.js before it gets to pouchdb-adapter-asyncstorage/src/get_attachment.js, where it wreaks havoc. Have to patch it to `false` in that file or... find out how to make it always false in PouchDB somewhere.
- Can't patch it in PouchDB because the docs explicitly say that `getAttachment()` always returns a blob. I don't think they will be happy being asked to change that, because then all of their plugins would have to support non-binary queries :( But now the plugins all have to assume that the `binary` option actually means "binary if possible otherwise not". adapter-asyncstorage actually uses this option, but it can't have ever worked! The docs even say that it is supposed to be for RN, so why...?
- URL.createObjectURL(blob) doesn't work in RN; changed to `'data:image/png;base64,' + blob`
- `FileReader.readAsArrayBuffer` polyfill is still required for the HTTP attachment pulling

***

Quick overview of the fixes we have:

PR's sent:

1. RxDB doesn't handle missing revision IDs: pubkey/rxdb#1347
2. PouchDB's feature detection logic not working for HN: pouchdb/pouchdb#7862

Local patches:

1. Fix for RxDB not handling missing revision IDs
2. Fix for pouchdb-adapter-asyncstorage trying to respect `options.binary`,
(PouchDB always sets to `true`, must be `false` to prevent errors in RN).

The PouchDB feature detection logic is also fixed by simply setting
`process.browser = true;` for now, since it was even easier than a patch and
should be temporary.

Finally, we also keep the polyfill for `FileReader.prototype.readAsArrayBuffer`,
which is needed for reading the binary attachment data from the server.

***

So at the end of the day, I have two GH tickets and two local patches. It may be better to open a third regarding the hardcoding of `options.binary` to `true` in PouchDB.core's `getAttachments()`, but I wouldn't now how to solve it. I just know it's problematic. In the longer wrong I would like both PR's accepted, and both patches removed (one fixed by the PR, the other by switching to an RN sqlite adapter).
garfieldnate added a commit to garfieldnate/flashcards that referenced this pull request Aug 2, 2019
What an adventure today was! I spent the whole day, minus lunch and a short
meeting with Rupesh, debugging this. If you want to know how long that was,
check the commit timestamp :) I did learn how to debug from VS code, which is
cool, although apparently I may have to restart my computer once in a while T_T

Here's a chronological list of everything I tried or investigated and the notes
I wrote while doing that:

- going back to previous commits and redoing some work
- resetting DB
- resetting device
- pushing data from app and trying to reload it (pushing only works sometimes)
- checking that other webapps are not running (shouldn't matter, but I'm desperate)
- Updating dependencies (including Expo and even node!)
- pouchdb-adapter-http's fetchJSON keeps returning this stuff:
`{"\_40":0,"\_65":0,"\_55":null,"\_72":null}` (oh wait, that's what a Promise
looks like for some reason!). Actually fetchJSON returns stuff correctly.
Goes to http://localhost:5984/flashcards/_changes?style=all_docs&since=0&limit=100,
gets back big thing with all flashcards.
- made pouchdb adapters same version as RxDB's pouchdb core dependency
- changing order of RxDB.plugin calls
- had to downgrade XDL for debugging to work in VSCode. See
microsoft/vscode-react-native#1060
- debugging (at least with VS code plugin) is difficult; expo app freezes when
I try to restart debugging
- I've been having trouble with my headphones giving crappy sound over bluetooth
lately. Figured out that closing the home screen in the Simulator actually brings
the sound quality back! Although it's still stronger in one ear. It's extra
weird because you can't even turn bluetooth connections in the simulator.
- do `yarn add pouchdb-debug` and then find "comment in to debug" in
RxDB/pouch-db.js. Did nothing.
- Copied that section into my own code and now I can see PouchDB debug statements!

    const pouchdbDebug = require('pouchdb-debug');
    RxDB.plugin(pouchdbDebug);
    RxDB.PouchDB.debug.enable('*');

- Apparently revs can be missing in change sequences, so patch RxDB/utils.js:

    export function getHeightOfRevision(revString) {
      if (!revString) {
        return 0;
      }
      const first = revString.split('-')[0];
      return parseInt(first);
    }

- Set process.browser = true globally to make sure pouchdb-adapter-http uses response.blob() instead of .buffer()
- Sent two PR's regarding these issues
- Man, you have to be really careful with the vscode RN/Expo debugger. I ended
up spawning lots of sleeping/waiting processes that I couldn't kill, and then
debugging just wouldn't work anymore, I think because all the metro packagers
were trying to connect to the client and do the same thing. Just had to restart
my computer T_T
- Getting  `Creating blobs from 'ArrayBuffer' and 'ArrayBufferView' are not
supported`, but can't get the call stack. Annoying pouchdb reports this as not
found instead of error :( The attachments are definitely being found.
- Tracked message to react-native/Libraries/Blob/BlobManager.js in the Blob constructor.
- calling PouchDB's getAttachment() with options = { binary: false } gets ignored because PouchDB tries to do the most "efficient" thing on its own. Gets set to "true" in node_modules/pouchdb-core/lib/index.js before it gets to pouchdb-adapter-asyncstorage/src/get_attachment.js, where it wreaks havoc. Have to patch it to `false` in that file or... find out how to make it always false in PouchDB somewhere.
- Can't patch it in PouchDB because the docs explicitly say that `getAttachment()` always returns a blob. I don't think they will be happy being asked to change that, because then all of their plugins would have to support non-binary queries :( But now the plugins all have to assume that the `binary` option actually means "binary if possible otherwise not". adapter-asyncstorage actually uses this option, but it can't have ever worked! The docs even say that it is supposed to be for RN, so why...?
- URL.createObjectURL(blob) doesn't work in RN; changed to `'data:image/png;base64,' + blob`
- `FileReader.readAsArrayBuffer` polyfill is still required for the HTTP attachment pulling

***

Quick overview of the fixes we have:

PR's sent:

1. RxDB doesn't handle missing revision IDs: pubkey/rxdb#1347
2. PouchDB's feature detection logic not working for HN: pouchdb/pouchdb#7862

Local patches:

1. Fix for RxDB not handling missing revision IDs
2. Fix for pouchdb-adapter-asyncstorage trying to respect `options.binary`,
(PouchDB always sets to `true`, must be `false` to prevent errors in RN).

The PouchDB feature detection logic is also fixed by simply setting
`process.browser = true;` for now, since it was even easier than a patch and
should be temporary.

Finally, we also keep the polyfill for `FileReader.prototype.readAsArrayBuffer`,
which is needed for reading the binary attachment data from the server.

***

So at the end of the day, I have two GH tickets and two local patches. It may be better to open a third regarding the hardcoding of `options.binary` to `true` in PouchDB.core's `getAttachments()`, but I wouldn't now how to solve it. I just know it's problematic. In the longer wrong I would like both PR's accepted, and both patches removed (one fixed by the PR, the other by switching to an RN sqlite adapter).

What a night!
garfieldnate added a commit to garfieldnate/flashcards that referenced this pull request Aug 3, 2019
What an adventure today was! I spent the whole day, minus lunch and a short
meeting with Rupesh, debugging this. If you want to know how long that was,
check the commit timestamp :) I did learn how to debug from VS code, which is
cool, although apparently I may have to restart my computer once in a while T_T

Here's a chronological list of everything I tried or investigated and the notes
I wrote while doing that:

- going back to previous commits and redoing some work
- resetting DB
- resetting device
- pushing data from app and trying to reload it (pushing only works sometimes)
- checking that other webapps are not running (shouldn't matter, but I'm desperate)
- Updating dependencies (including Expo and even node!)
- pouchdb-adapter-http's fetchJSON keeps returning this stuff:
`{"\_40":0,"\_65":0,"\_55":null,"\_72":null}` (oh wait, that's what a Promise
looks like for some reason!). Actually fetchJSON returns stuff correctly.
Goes to http://localhost:5984/flashcards/_changes?style=all_docs&since=0&limit=100,
gets back big thing with all flashcards.
- made pouchdb adapters same version as RxDB's pouchdb core dependency
- changing order of RxDB.plugin calls
- had to downgrade XDL for debugging to work in VSCode. See
microsoft/vscode-react-native#1060
- debugging (at least with VS code plugin) is difficult; expo app freezes when
I try to restart debugging
- I've been having trouble with my headphones giving crappy sound over bluetooth
lately. Figured out that closing the home screen in the Simulator actually brings
the sound quality back! Although it's still stronger in one ear. It's extra
weird because you can't even turn bluetooth connections in the simulator.
- do `yarn add pouchdb-debug` and then find "comment in to debug" in
RxDB/pouch-db.js. Did nothing.
- Copied that section into my own code and now I can see PouchDB debug statements!

    const pouchdbDebug = require('pouchdb-debug');
    RxDB.plugin(pouchdbDebug);
    RxDB.PouchDB.debug.enable('*');

- Apparently revs can be missing in change sequences, so patch RxDB/utils.js:

    export function getHeightOfRevision(revString) {
      if (!revString) {
        return 0;
      }
      const first = revString.split('-')[0];
      return parseInt(first);
    }

- Set process.browser = true globally to make sure pouchdb-adapter-http uses response.blob() instead of .buffer()
- Sent two PR's regarding these issues
- Man, you have to be really careful with the vscode RN/Expo debugger. I ended
up spawning lots of sleeping/waiting processes that I couldn't kill, and then
debugging just wouldn't work anymore, I think because all the metro packagers
were trying to connect to the client and do the same thing. Just had to restart
my computer T_T
- Getting  `Creating blobs from 'ArrayBuffer' and 'ArrayBufferView' are not
supported`, but can't get the call stack. Annoying pouchdb reports this as not
found instead of error :( The attachments are definitely being found.
- Tracked message to react-native/Libraries/Blob/BlobManager.js in the Blob constructor.
- calling PouchDB's getAttachment() with options = { binary: false } gets ignored because PouchDB tries to do the most "efficient" thing on its own. Gets set to "true" in node_modules/pouchdb-core/lib/index.js before it gets to pouchdb-adapter-asyncstorage/src/get_attachment.js, where it wreaks havoc. Have to patch it to `false` in that file or... find out how to make it always false in PouchDB somewhere.
- The other error that must be avoided in pouchdb-adapter-http is
`Cannot set property type of #<Blob> which has only a getter`. I guess
`Blob.type` is readonly in RN.
- Can't patch it in PouchDB because the docs explicitly say that `getAttachment()` always returns a blob. I don't think they will be happy being asked to change that, because then all of their plugins would have to support non-binary queries :( But now the plugins all have to assume that the `binary` option actually means "binary if possible otherwise not". adapter-asyncstorage actually uses this option, but it can't have ever worked! The docs even say that it is supposed to be for RN, so why...?
- URL.createObjectURL(blob) doesn't work in RN; changed to `'data:image/png;base64,' + blob`
- `FileReader.readAsArrayBuffer` polyfill is still required for the HTTP attachment pulling

***

Quick overview of the fixes we have:

PR's sent:

1. RxDB doesn't handle missing revision IDs: pubkey/rxdb#1347
2. PouchDB's feature detection logic not working for HN: pouchdb/pouchdb#7862

Local patches:

1. Fix for RxDB not handling missing revision IDs
2. Fix for pouchdb-adapter-asyncstorage trying to respect `options.binary`,
(PouchDB always sets to `true`, must be `false` to prevent errors in RN).

The PouchDB feature detection logic is also fixed by simply setting
`process.browser = true;` for now, since it was even easier than a patch and
should be temporary.

Finally, we also keep the polyfill for `FileReader.prototype.readAsArrayBuffer`,
which is needed for reading the binary attachment data from the server.

***

So at the end of the day, I have two GH tickets and two local patches. It may be better to open a third regarding the hardcoding of `options.binary` to `true` in PouchDB.core's `getAttachments()`, but I wouldn't now how to solve it. I just know it's problematic. In the longer wrong I would like both PR's accepted, and both patches removed (one fixed by the PR, the other by switching to an RN sqlite adapter).

What a night!
@garfieldnate garfieldnate force-pushed the patch-1 branch 2 times, most recently from 1806c2f to a2761d6 Compare August 3, 2019 18:11
@garfieldnate
Copy link
Contributor Author

@daleharvey Following your suggestion I revised the commit.

@daleharvey
Copy link
Member

Codewise I like the look of the changes, but need to investigate the test failures, they are genuine, cheers

response.buffer() is not supported at all, and blob.type is readonly in
ReactNative. Change the limited node/browser detection to (hopefully)
future-proof feature detection instead.

Errors fixed in RN:

    Creating blobs from 'ArrayBuffer' and 'ArrayBufferView' are not
    supported

    Cannot set property type of #<Blob> which has only a getter at

Fixes pouchdb#7688 and pouchdb#7727
@garfieldnate
Copy link
Contributor Author

@daleharvey All fixed.

@daleharvey
Copy link
Member

Awesome thanks, looks good, we usually require tests but we dont test in reactnative so I think thats fine for now, cheers

@daleharvey daleharvey merged commit 8351070 into pouchdb:master Aug 7, 2019
@garfieldnate
Copy link
Contributor Author

I've seen you comment before that you do not plan on supporting react native because you believe everything can be done in plugins. Is that still the case? Getting PouchDB to work on RN was such a nightmare for me, but it's such convenient tech that it's hard not to try.

If I opened another PR to add testing in react native, would you accept it?

@garfieldnate garfieldnate deleted the patch-1 branch August 9, 2019 06:25
sto3psl pushed a commit to sto3psl/pouchdb that referenced this pull request Mar 10, 2021
…ss in ReactNative

response.buffer() is not supported at all, and blob.type is readonly in
ReactNative. Change the limited node/browser detection to (hopefully)
future-proof feature detection instead.

Errors fixed in RN:

    Creating blobs from 'ArrayBuffer' and 'ArrayBufferView' are not
    supported

    Cannot set property type of #<Blob> which has only a getter at

Fixes pouchdb#7688 and pouchdb#7727
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.

Sync attachments from CouchDB : Error in fetchData function response.buffer() not a function
2 participants