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

React-Native support #313

Open
Gerharddc opened this issue Jan 10, 2018 · 41 comments
Open

React-Native support #313

Gerharddc opened this issue Jan 10, 2018 · 41 comments

Comments

@Gerharddc
Copy link

I would like to use this library together with something like https://github.com/craftzdog/react-native-sqlite-2 (which provides a WebSQL compatible interface through SQLite) to get Indexeddb support on React-Native.

I was hopeing this would be easy and that something like the following should work:

import SQLite from 'react-native-sqlite-2';
import setGlobalVars from 'indexeddbshim';

let idb = {};
setGlobalVars(idb, { win: SQLite });
var { indexedDB, IDBKeyRange } = idb;

But this fails when trying to import indexeddbshim with the following error:
screenshot_1515598757

I think that maybe it is trying to do some fancy stuff because it might think it is running on Node. I passed the SQLite object as win in the config because it would not find openDatabase any other way.

What can I try to make this work?

@pwnbreak
Copy link

I managed to make it (almost) work with this:

import SQLite from 'react-native-sqlite-2';
import setGlobalVars from 'indexeddbshim/src/setGlobalVars';

import Dexie from 'dexie';

const win = {};

setGlobalVars(
  win,
  {
    checkOrigin: false,
    win: SQLite,
    deleteDatabaseFiles: false,
    useSQLiteIndexes: true,
  }
);

class MyDexie extends Dexie {
  constructor(name) {
    super(name, {
      indexedDB: win.indexedDB,
      IDBKeyRange: win.IDBKeyRange,
    });
  }
}

I say almost because Babel seems to be ignoring an arrow function in the output bundle:

Original:

Object.defineProperty(IDBVersionChangeEvent, Symbol.hasInstance, {
    value: obj => util.isObj(obj) && 'oldVersion' in obj && typeof obj.defaultPrevented === 'boolean'
});

Output:

Object.defineProperty(IDBVersionChangeEvent, typeof Symbol === 'function' ? Symbol.hasInstance : '@@hasInstance', {
    value: obj => util.isObj(obj) && 'oldVersion' in obj && typeof obj.defaultPrevented === 'boolean'
});

So there is a syntax error in Android, but at least it works in remote debugger for me.

@Gerharddc
Copy link
Author

Gerharddc commented Jan 17, 2018

@nizkp Unfortunately testing with the remote debugger is not an option in this case because that makes all JS execute remotely in Chrome instead of through React-Native's own runtime. What that means is that normal Indexeddb should even work there without the shim.

Getting this Babel problem fixed could solve everything though.

@pwnbreak
Copy link

True, but it's actually working (data is stored in mobile via react-native-sqlite-2). But of course it's not useful if it can't run in the old Android JSC Engine.
I think the problem must come from some custom Babel transform that react-native must be doing, or doing transforms in the wrong order. I'll try to investigate a little bit.

@brettz9
Copy link
Collaborator

brettz9 commented Jan 20, 2018

Though the ES2015 preset should normally convert, FWIW, there is a Babel plugin for arrow functions: https://babeljs.io/docs/plugins/transform-es2015-arrow-functions/

@jeanregisser
Copy link

jeanregisser commented Jan 30, 2018

I'm using IndexedDBShim successfully with Dexie in a React Native project (iOS and Android) with the following code:

// file: configureDB.js

export default function configureDB() {
  // Make indexeddbshim happy with React Native's environment
  if (global.window.navigator.userAgent === undefined) {
    global.window.navigator = { ...global.window.navigator, userAgent: '' };
  }
  // Import after RN initilization otherwise we get an exception about process not being defined
  const SQLite = require('react-native-sqlite-2').default;
  global.window.openDatabase = SQLite.openDatabase;
  // babel-polyfill is Needed on Android otherwise indexeddbshim complains
  // about Object.setPrototypeOf missing
  require('babel-polyfill');
  require('indexeddbshim');
  global.shimIndexedDB.__setConfig({ checkOrigin: false });

  const Dexie = require('dexie');
  const db = new Dexie('my_db');

  // schema
  db.version(1).stores({
    friends: 'name,shoeSize'
  });

  return db;
}

// file: index.js
import React from 'react';
import {
  AppRegistry,
  View,
} from 'react-native';
import configureDB from './configureDB';

function setup() {
  db = configureDB();
  // [...]

  class App extends React.Component {
    render() {
      return (
          <View style={styles.container}>
             {...}
          </View>
      );
    }
  }
  return App;
}

AppRegistry.registerComponent('MyApp', setup);

This is a quite hackish and I wish we would have proper support for React Native directly in IndexedDBShim. Haven't found the time to make this happen myself though.

Also this setup is working (mostly) for my use case up until indexeddbshim@3.1.0.

I say mostly as I have had incorrect results when querying multi-entry indexes with Dexie.

I thought those issues might be fixed by upgrading IndexedDBShim.
But starting with indexeddbshim@3.2.0 I get incorrect serialization/deserialization of objects containining undefined properties.

So I had to keep using v3.1.0.

Example:

// When saving the following object with IndexedDBShim: 
{ foo: "my value", bar: undefined }
// It is being stored within the sqlite DB as
{"foo":"my value","bar":null,"$types":{"bar":"userObject"}}
// And then when read back from the DB, I get this:
{ foo: "my value", bar: {} }

I tracked it down to typeson issues in this particular React Native env within the minified indexeddbshim bundle.
Though when I manually run typeson stringifySync and parse with the same registered types in React Native I don't get any issue:

var Typeson = require('typeson');
var reg = require('typeson-registry');

// Simulates what happens in https://github.com/axemclion/IndexedDBShim/blob/6c4ed0694edefc37ce814140240d6f58f1e0f316/src/Sca.js

var structuredCloning = reg.presets.structuredCloningThrowing;

function traverseMapToRevertToLegacyTypeNames (obj) {
    if (Array.isArray(obj)) {
        return obj.forEach(traverseMapToRevertToLegacyTypeNames);
    }
    if (obj && typeof obj === 'object') { // Should be all
        Object.entries(obj).forEach(([prop, val]) => {
            if (prop in newTypeNamesToLegacy) {
                const legacyProp = newTypeNamesToLegacy[prop];
                delete obj[prop];
                obj[legacyProp] = val;
            }
        });
    }
}

var newTypeNamesToLegacy = {
    IntlCollator: 'Intl.Collator',
    IntlDateTimeFormat: 'Intl.DateTimeFormat',
    IntlNumberFormat: 'Intl.NumberFormat',
    userObject: 'userObjects',
    undef: 'undefined',
    negativeInfinity: 'NegativeInfinity',
    nonbuiltinIgnore: 'nonBuiltInIgnore',
    arraybuffer: 'ArrayBuffer',
    blob: 'Blob',
    dataview: 'DataView',
    date: 'Date',
    error: 'Error',
    file: 'File',
    filelist: 'FileList',
    imagebitmap: 'ImageBitmap',
    imagedata: 'ImageData',
    infinity: 'Infinity',
    map: 'Map',
    nan: 'NaN',
    regexp: 'RegExp',
    set: 'Set',
    int8array: 'Int8Array',
    uint8array: 'Uint8Array',
    uint8clampedarray: 'Uint8ClampedArray',
    int16array: 'Int16Array',
    uint16array: 'Uint16Array',
    int32array: 'Int32Array',
    uint32array: 'Uint32Array',
    float32array: 'Float32Array',
    float64array: 'Float64Array'
};

// console.log('StructuredCloning1', JSON.stringify(structuredCloning));
traverseMapToRevertToLegacyTypeNames(structuredCloning);
// console.log('StructuredCloning2', JSON.stringify(structuredCloning));

var TSON = new Typeson().register(structuredCloning);

var r = TSON.stringifySync({foo: "my value", bar: undefined });
console.log('==result', r);

var r2 = TSON.parse(r);
console.log('==result2', r2);
// it outputs the expected data:
==result {"foo":"my value","bar":null,"$types":{"bar":"undefined"}}
==result2 { foo: 'my value', bar: undefined }

So this effectively suggests the issue is within the minified IndexedDBShim bundle imported the way you see it above.
Again proper support for React Native in IndexedDBShim would probably fix those side effects.

Hope it will help someone else.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 2, 2018

Great to have this info, thanks! I don't know when I may be able to get to looking at this to see what we can do to fix. Please keep us updated if you would if things change on the React Native side...

@brettz9
Copy link
Collaborator

brettz9 commented Feb 10, 2018

@jeanregisser , can I ask if you also tested the non-minified bundle and whether that had problems for you too?

@brettz9
Copy link
Collaborator

brettz9 commented Feb 10, 2018

I'd really like to better understand the issues in the latest IndexedDBShim regarding serialization/deserialization of objects with undefined as Typeson and typeson-registry tests are all passing (as are IndexedDBShim tests).

I suppose there is a chance though that the changes to Sca.js to revert legacy type names are not properly reverting pre-existing data with undefined?

@vladikoff
Copy link

vladikoff commented Apr 8, 2018

This also works:

// IndexedDB hackery begins
Object.setPrototypeOf = Object.setPrototypeOf || function(obj, proto) {
  obj.__proto__ = proto;
  return obj;
}
// Make indexeddbshim happy with React Native's environment
if (global.window.navigator.userAgent === undefined) {
  global.window.navigator = { ...global.window.navigator, userAgent: '' };
}
const SQLite = require('react-native-sqlite-2').default;
global.window.openDatabase = SQLite.openDatabase;
require('indexeddbshim');
global.shimIndexedDB.__setConfig({ checkOrigin: false });

if you don't wanna do require('babel-polyfill');

shrugs

brettz9 added a commit to brettz9/IndexedDBShim that referenced this issue Apr 9, 2018
…porarily

    circumvent current limitations in Babel until
    <babel/babel#5978> addressed; (see also
    <indexeddbshim#311 (comment)>);
    fixes indexeddbshim#311
- Linting: Expand ESLint file coverage and apply minor linting fix to test file
- npm: Update `eventtargeter` to avoid automatic `Object.setPrototypeOf`
    calls (make conditional on `CFG.fullIDLSupport`); fixes indexeddbshim#313
- npm: Bump to 3.6.0
brettz9 added a commit to brettz9/IndexedDBShim that referenced this issue Apr 9, 2018
…porarily

    circumvent current limitations in Babel until
    <babel/babel#5978> addressed; (see also
    <indexeddbshim#311 (comment)>);
    fixes indexeddbshim#311
- Linting: Expand ESLint file coverage and apply minor linting fix to test file
- Testing: Update web-platform-tests and our tests accordingly
- npm: Update `eventtargeter` to avoid automatic `Object.setPrototypeOf`
    calls (make conditional on `CFG.fullIDLSupport`); fixes indexeddbshim#313
- npm: Bump to 3.6.0
@brettz9 brettz9 closed this as completed in fc1c0f3 Apr 9, 2018
@brettz9
Copy link
Collaborator

brettz9 commented Apr 9, 2018

In 3.6.0, the issue with setPrototypeOf should now be fixed. (Just don't set fullIDLSupport to true as tht will use Object.setPrototypeOf).

Note that as far as the previous comment, you don't need to polyfill userAgent if you invoke setGlobalVars. In that call, you can also add win: SQLite to your config (and you can forego a separate call to __setConfig by passing your config as the second argument to setGlobalVars).

But the latter example was a helpful reminder to check setPrototypeOf (which a dependency had been unconditionally adding, now fixed).

Anyways, feel free to report back on whether the fix works for everybody...

@vladikoff
Copy link

@brettz9 doing this:


const setGlobalVars = require('indexeddbshim');
setGlobalVars({
  win: SQLite
}, { checkOrigin: false });

image

Am I missing something?

@vladikoff
Copy link

Doing a log like this:

console.log('setGlobalVars', require('indexeddbshim'));

produces:

04-09 19:13:36.335  4379  4784 I ReactNativeJS: 'setGlobalVars', {}

so the setGlobalVars is not exported it seems

@brettz9
Copy link
Collaborator

brettz9 commented Apr 10, 2018

@vladikoff : What do you get if you try this instead:

import setGlobalVars from 'indexeddbshim/src/setGlobalVars';
console.log(setGlobalVars);

@vladikoff
Copy link

@brettz9 the loading via /src/setGlobalVars doesn't work.

With import setGlobalVars from 'indexeddbshim'; I get this:

The way import statements are loaded, it seems that even having

if (global.window.navigator.userAgent === undefined) {
  global.window.navigator = { ...global.window.navigator, userAgent: '' };
}

above won't help to save it.

@brettz9
Copy link
Collaborator

brettz9 commented Apr 11, 2018

How about

const setGlobalVars = require('indexeddbshim/dist/indexeddbshim-noninvasive.js');
console.log(setGlobalVars);

Btw, I can easily push a fix to check the existence of userAgent, just trying to get as much info first as possible...

@brettz9
Copy link
Collaborator

brettz9 commented Apr 11, 2018

(If you wanted to go the import route, you'd need to find a way for importing module's like the Rollup plugin "rollup-plugin-node-builtins" (or equivalent for that environment) as our source is importing and relies on the Node module path, even for non-Node usage. That is the reason for the first error you reported in your last message.)

@zaptrem
Copy link

zaptrem commented Feb 7, 2020

So what's the full currently established way of doing this?

@brettz9
Copy link
Collaborator

brettz9 commented Feb 8, 2020

@zaptrem : See (or answer) my comments above to @vladikoff

@zaptrem
Copy link

zaptrem commented Feb 8, 2020 via email

@swittk
Copy link

swittk commented Feb 18, 2020

Also interested in using an IndexedDB shim on React Native/Expo. (Just realized that Cloud Firestore wasn't doing Offline Persistence due to the lack of it)

I've tried doing what was mentioned in this thread firebase/firebase-js-sdk#436 by @zwily (Gist here : https://gist.github.com/zwily/e9e97e0f9f523a72c24c7df01d889482), and sure enough, like others, it seems to work on iOS, and crashes on Android.

The crash shows a message I've identified to come from the function IDBObjectStore.prototype.__validateKeyAndValueAndCloneValue (inside '/indexeddbshim/dist/indexeddbshim-noninvasive.js')

and the error is thrown after the following checks

var _clonedValue = Sca.clone(value);

    key = Key.extractKeyValueDecodedFromValueUsingKeyPath(_clonedValue, me.keyPath); // May throw so "rethrow"

...

    if (key.failure) {

      if (!cursorUpdate) {

        if (!me.autoIncrement) {

          throw (0, _DOMException.createDOMException)('DataError', 'Could not evaluate a key from keyPath and there is no key generator'); 

        //--------> This here is the error that's happening on android

        }

I'm wondering if anyone could help shed some light on what is happening in this part of the code. The source is kind of full of semi-minified code and it's kind of hard to trace and understand them all.
I also don't understand what seems to be causing the Android react native implementation to behave differently to the iOS version? Is there some class that is referenced somewhere that isn't fully implemented?

@zaptrem
Copy link

zaptrem commented Feb 18, 2020

Also interested in using an IndexedDB shim on React Native/Expo. (Just realized that Cloud Firestore wasn't doing Offline Persistence due to the lack of it)

I've tried doing what was mentioned in this thread firebase/firebase-js-sdk#436 by @zwily (Gist here : https://gist.github.com/zwily/e9e97e0f9f523a72c24c7df01d889482), and sure enough, like others, it seems to work on iOS, and crashes on Android.

The crash shows a message I've identified to come from the function IDBObjectStore.prototype.__validateKeyAndValueAndCloneValue (inside '/indexeddbshim/dist/indexeddbshim-noninvasive.js')

and the error is thrown after the following checks

var _clonedValue = Sca.clone(value);

    key = Key.extractKeyValueDecodedFromValueUsingKeyPath(_clonedValue, me.keyPath); // May throw so "rethrow"

...

    if (key.failure) {

      if (!cursorUpdate) {

        if (!me.autoIncrement) {

          throw (0, _DOMException.createDOMException)('DataError', 'Could not evaluate a key from keyPath and there is no key generator'); 

        //--------> This here is the error that's happening on android

        }

I'm wondering if anyone could help shed some light on what is happening in this part of the code. The source is kind of full of semi-minified code and it's kind of hard to trace and understand them all.
I also don't understand what seems to be causing the Android react native implementation to behave differently to the iOS version? Is there some class that is referenced somewhere that isn't fully implemented?

I think Android uses some special Facebook implementation of the JS Engine while iOS is required to use JavaScriptCore/WebKit.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 19, 2020

Thank you for reporting on what you have tried and where things are going wrong (the example code you have at https://gist.github.com/zwily/e9e97e0f9f523a72c24c7df01d889482 looks pretty sound, and may be helpful to others. (FWIW, I have added in a branch some code to avoid the need for the userAgent setting portion.) I'd like to help with this issue, but don't have time to dive into React/React-Native myself.

As far as the minified code, probably what you are seeing is from typeson/typeson-registry, the code responsible for encoding various object types into strings that can be stored in sqlite and then decoding them back to objects (to use the technical terminology, those that can be cloned by the "structured cloning algorithm"). I've started a branch with some code to move from grunt-browserify to grunt-rollup, as I think it is easier to work with, and should, if I can get it done, help me tweak the settings to avoid requiring the minified version in our non-minified builds (hopefully source maps would avoid the need, however).

The code you have excerpted (and much of the codebase) tries to follow the spec pretty closely, even in terminology as possible.

The code beginning Key.extractKeyValueDecodedFromValueUsingKeyPath is detailed at https://w3c.github.io/IndexedDB/#extract-a-key-from-a-value-using-a-key-path .

The subsequent checks are a part of our internal __validateKeyAndValueAndCloneValue which is called by IDBObjectStore add or put (and also by IDBCursor#update but that wouldn't get past the !cursorUpdate check, so looks like that is not it in your case). Presumably, you are therefore using an object store add or put, and Key.extractKeyValueDecodedFromValueUsingKeyPath returns a failure.

A failure can occur when the method evaluateKeyPathOnValueToDecodedValue is called.

If the key path is an array, the array items will be individually checked and if any fail, that call will fail. The other cases that can fail can be seen at https://github.com/axemclion/IndexedDBShim/blob/master/src/Key.js#L609-L613 (the lines preceding also set value).

These lines basically tell us that if the keypath sequence can't find a component in the sequence, then there will be a failure.

The issue could occur with _clonedValue if the typeson/typeson-registry cloning doesn't work in that environment for some reason. I'd therefore begin by introspecting to see what the value is. If it is a valid object that you expect at that point, then see what me.keyPath is, and whether it makes sense that that key path cannot be found on the given object. Without the object store being auto-increment, a DOMException is thrown as there is no way to determine what the key should be.

You can get usage help on Stackoverflow, but if there is a problem where cloning has not occurred properly, you can report that here (though it may need to go to typeson/typeson-registry, though I should also be able to help over there if that is the case).

@brettz9 brettz9 reopened this Feb 19, 2020
@swittk
Copy link

swittk commented Feb 19, 2020

Thanks for the helpful info :)
I've been trying to identify the issue this past afternoon and was looking into evaluateKeyPathOnValueToDecodedValue's implementation, and even considering testing the native Array.isArray implementation on Android haha.

Scratch that : _clonedValue and value are the same.

What I've found is that when state updates are performed (I guess the add, or put function is called), the value parameter is simply an empty object!
This is what is logged after
var _clonedValue = Sca.clone(value);
(Inside IDBObjectStore.prototype.__validateKeyAndValueAndCloneValue)
Just before the crash.
_clonedValue: {}
value: {}
keypath: batchId

Update : Empty object only occurs when 'add' is called; 'put' calls have objects with valid, expected keyPath property inside them.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 19, 2020

@swittk : I am guessing it is a bug in typeson or typeson-registry with the type of data you are trying to clone.

What version of indexeddbshim are you using? I would update to 6.0.1 (or current master is ok). There were a couple typeson-related upgrades which fixed one or two issues that also led to undefined. There's also one known issue left that has this problem: dfahlander/typeson#11 .

However, it is puzzling to me that a simple object should have this issue. Is the value object {"clientId":"YI3pQPn90MSASrsyUvuR","updateTimeMs":1582120408087,"networkEnabled":true,"inForeground":false} just a plain object or are these on some special object type which also has those properties?

Since those encode/decode functions are just wrappers for typeson-registry with the structuredCloningThrowing preset, you will probably need to find some way to debug within or against typeson/typeson-registry. I don't know which workaround may work in your environment until I may get a non-minified version shipped, e.g., inlining the typeson/typeson-registry code, importing from its latest version, or using require statements (or script tags)--whatever React Native may support (or just include typeson-registry outside of indexeddbshim in React Native and see what happens when you clone your object there). Complicating this, I only recently fixed an issue and made a release for typeson-registry whereby it was not bundling the index.js source which is needed when requiring an ESM (as opposed to UMD) environment. While I have added reference to this new version in my indexeddbshim branch I'm working on, that is not yet released.

@swittk
Copy link

swittk commented Feb 19, 2020

I'm so sorry, in my prior comment I wasn't logging the object correctly (logged clonedValue instead of _clonedValue).

I've since edited the post. I've found that value and _clonedValue are identical in all cases (So Sca.clone works, no problem).

However I've found that the crash occurs when the value provided is an empty object ( {} ), while keyPath is some value (keypath="batchId" to be exact; I guess it's something from Firestore)

At first start of the application; one 'add' operation is successful
(This is logged in IDBObjectStore.prototype.add)
key: undefined
value:{"fbase_key":"__sak674766500","value":"674766500"}

_clonedValue, value, and keypath (fbase_key) are as expected inside __validateKeyAndValueAndCloneValue

after that there are multiple calls to 'put', which seem to behave normally as expected

Then just before the crash the condition referenced before happens
(This is logged in IDBObjectStore.prototype.add)
key: undefined
value:{}

_clonedValue and value are {}, but keypath is 'batchId'

I'm not sure if I understand this correctly, but it seems either batchId came from somewhere unexpected within firebase, or the object itself isn't really a typical object.

However, I've compared the logs to the react-native iOS version (which do work) and found that the same log occurs in IDBObjectStore.add, however it doesn't crash...?

key:undefined, value:{}
In __validateKeyAndValueAndCloneValue : _clonedValue: {}, value: {}, keypath: batchId

In the iOS version, after the curious 'add' with the empty object call, normal 'put' calls are executed after that with no issue, and no crash occurs.

update:
Interestingly in iOS (where things don't crash); the add call with the empty object is immediately followed by a put call with a large object containing what looks like the data to be added. In this put call, me.keyPath is also "batchId", but the value object in the put call does have batchId inside of it.

This behavior probably coincides with creating a new document with no data in firestore then assigning data to it (to be specific in my case; creating a new DocumentReference, getting the id of it, then calling .set(value) to the DocumentReference). However, I still do not fully understand how batchId came to exist while the value object itself is completely empty..

@swittk
Copy link

swittk commented Feb 19, 2020

I've since come to discover that the iOS and Android versions return identical results, and fail the same Key.extractKeyValueDecodedFromValueUsingKeyPath test (both return {"failure":true}).

The only difference is in the autoIncrement property; iOS has it being true, while Android has it being false, and thus it chugs along on one and fails on the other.

I'm wondering if this property (autoIncrement) is configurable with the current API. Trying to set it with setGlobalVars(window, {autoIncrement:true}) shows autoIncrement is not a valid configuration property.

Update: Seems autoIncrement is part of the firestore's invocation of createObjectStore, and looking in the firestore/firebase source's minified code; all instances of autoIncrement are set to !0 (true). So I'm not sure why the autoIncrement check is showing false here (on the me.autoIncrement check).

@swittk
Copy link

swittk commented Feb 20, 2020

Just found another interesting thing in case it would help; I logged storeProperties.autoInc inside IDBObjectStore.__createInstance and the results were surprising;

on iOS almost all calls are autoInc:false, with very, very few calls having autoInc:true
on Android it is the exact inverse; autoInc: true for almost all logs, with very few autoInc: false (and the last autoInc:false value is just right before the crash)

@swittk
Copy link

swittk commented Feb 20, 2020

Screen Shot 2563-02-20 at 08 00 25

The logs show that the autoInc value provided to IDBObjectStore.__createInstance are the exact opposite in these two platforms .__. I don't know where to look for next; I'm guessing that the true/false values after the numerical values are due to the values being converted to Boolean somewhere, then copied around. But the initial numerical values being 0 instead of 1 and vice versa is confusing me.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 20, 2020

Seems autoIncrement is part of the firestore's invocation of createObjectStore, and looking in the firestore/firebase source's minified code; all instances of autoIncrement are set to !0 (true).

Looking at https://github.com/firebase/firebase-js-sdk/blob/542240d093ad54f3c8e5739dcb8449e8b9cab131/packages/firestore/src/local/indexeddb_schema.ts#L468-L470 , it appears that autoIncrement is missing from this case.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 20, 2020

Please note that as of version 6.1.0 just released, the code at https://gist.github.com/zwily/e9e97e0f9f523a72c24c7df01d889482#file-expo-firestore-persistence-hack-js-L31-L34 should no longer be needed.

I guess I'll keep this issue open in case someone can write up a section for a README explaining usage which is not tied to a particular library but may work for React Native (if such a thing is possible). But the README already details usage, so tbh, I'm not sure even this is necessary.

(The Firebase issue appears to be a bug in Firebase, so you might try submitting a PR which tries adding autoIncrement to that case where it is missing, and see if the devs might explore it further.)

@swittk
Copy link

swittk commented Feb 20, 2020

Hi.
First, I'd like to thank you so much for even looking up the firebase source on this issue.
One last thing I wonder though, would not providing a value to the parameter usually result in the parameter being false? Instead on Android the value seems to default to true, with the single autoIncrement: false call being the one right before the update. (On iOS the values are all false except for the one on the update, which I suspect is the correct behavior since it doesn't crash). Is there anywhere in this library where there are type conversions performed?

Screen Shot 2563-02-20 at 08 00 25

The logs show that the autoInc value provided to IDBObjectStore.createInstance are the exact opposite in these two platforms ..

@brettz9
Copy link
Collaborator

brettz9 commented Feb 22, 2020

I'm afraid I don't have much time to look into this now. IIUC, the default for autoIncrement is indeed false, but if two stores are made, one with it true and one without, maybe that is the cause of the difference.

OTOH, you may be right that the mirroring is due to something internal to IndexedDBShim and the environment. I think you need to find out where the calls to IDBObjectStore.__createInstance are coming from.

I did at least investigate the possibilities, and I suspect it is no. 2 below. What I might therefore try if I were you is changing the BOOLEAN reference within CREATE TABLE IF NOT EXISTS __sys__ (name BLOB, keyPath BLOB, autoInc BOOLEAN, indexList BLOB, currNum INTEGER) SQLite call to NUMERIC (what should be the default) and if that doesn't work, to INTEGER. You would need to be testing on freshly created data though rather than data created previously.

But in case that doesn't solve it, here is the full list of possibilities for the sources of the calls to IDBObjectStore.__createInstance:

  1. IDBDatabase.prototype.createObjectStore - I'd be surprised if it is from here as that is mostly just passing on any autoIncrement passed to it.
  2. From IDBDatabase.__createInstance (called within indexedDB.open). If this is the case, the issue is probably with JSON, or much more likely, an issue with how autoInc was stored in or retrieved from the database. While https://www.sqlite.org/datatype3.html#affinity_name_examples seems to me to suggest that BOOLEAN should be understandable in a CREATE TABLE statement with SQLite, per elsewhere on the page, SQLite does not actually have Boolean types itself, so maybe the Android implementation has an issue with this. Searching the web for Android, Boolean, and SQLite, I came across this: https://stackoverflow.com/questions/24604817/how-to-add-a-boolean-column-in-android-sqlite/24605762 though I don't know if the questioner was posting simply because they thought SQLite should not support booleans, or because Android actually has a problem with it in the CREATE TABLE syntax. (I do see a note in our source that "autoInc is numeric (0/1) on WinPhone" but that comment precedes the line in IDBObjectStore.__createInstance where the value coming to the function and setting autoIncrement is converted to a Boolean, so one would think this would not be a problem unless Android is storing the 0 and 1 as strings which I would doubt as this would lead to autoIncrement always being true--and that would be the expected behavior anyways as SQLite is only supposed to recognize the BOOLEAN type but not treat it as anything besides NUMERIC.)
  3. From IDBObjectStore.__clone
    1. As called by IDBTransaction.prototype.objectStore: This gets the store from __objectStores on the transaction's db property (coming from IDBDatabase). These stores can be set by IDBObjectStore.__createObjectStore (just using the store passed to it) or during creation of IDBDatabase (just by IDBObjectStore.__createInstance).
    2. As called by IDBObjectStore.__createObjectStore: This is only called by IDBDatabase.prototype.createObjectStore with the store supplied to the latter coming from IDBObjectStore.__createInstance based on properties, as mentioned above, which come pretty much directly from createObjectStore. So I think this is unlikely to be the source.

@swittk
Copy link

swittk commented Feb 23, 2020

Just a little update; I have found the solution :)
Yeah, I was looking into the code and logging basically the creation of the instances that have autoInc in them, and found that autoIncrement was stored in the sys data structure. That was as far as I got. So I figured it was something to do with the SQL query, but I couldn't find anything wrong with how it was written, so I left it at that to do some head scratching.

Then when I saw your latest comment, I figured that it might be worth trying to mess with that particular field ("autoInc BOOLEAN" in sys), that maybe Android SQLite really doesn't work well with Boolean

Long story short; I changed two things to make it work (all within the insertStoreInfo inside IDBObjectStore.__createObjectStore)

  1. Changed from BOOLEAN to NUMERIC
    tx.executeSql('CREATE TABLE IF NOT EXISTS __sys__ (name BLOB, keyPath BLOB, autoInc BOOLEAN, indexList BLOB, currNum INTEGER)', [], function () {

Changed BOOLEAN to NUMERIC
tx.executeSql('CREATE TABLE IF NOT EXISTS __sys__ (name BLOB, keyPath BLOB, autoInc NUMERIC, indexList BLOB, currNum INTEGER)', [], function () {

I did this first, but the resulting calls to IDBDatabase.__createInstance still showed storeProperties being the inverse of what was called to insertStoreInfo on first creation

(Example)
First run log : Shows insertStoreInfo being called (I logged inside insertStoreInfo the values [escapeSQLiteStatement(storeName), encodedKeyPath, store.autoIncrement, '{}', 1] )

insertStoreInfo ["firebaseLocalStorage",""fbase_key"",false,"{}",1]

Second run log : Shows IDBDatabase.__createInstance being called (I logged the storeProperties value)

storeProps:{"rowsAffected":0,"rows":{"_array":[{"name":"firebaseLocalStorage","keyPath":""fbase_key"","autoInc":1,"indexList":"{}"}],"length":1}}

So obviously, autoInc is still wrong.

So I did the next logical thing in case Booleans are really not playing nice with Android

  1. Casted the store.autoIncrement Boolean value to a Number
    tx.executeSql('INSERT INTO __sys__ VALUES (?,?,?,?,?)', [escapeSQLiteStatement(storeName), encodedKeyPath, Number(store.autoIncrement), '{}', 1], function () {

Seems dumb, right? I really didn't expect it to work, but hey.. the results speak for itself

First app launch :

insertStoreInfo ["firebaseLocalStorage",""fbase_key"",false,"{}",1]

Second app launch:

storeProps:{"rowsAffected":0,"rows":{"_array":[{"name":"firebaseLocalStorage","keyPath":""fbase_key"","autoInc":0,"indexList":"{}"}],"length":1}}

AutoIncrement is now correct! And firestore offline persistence is now working BEAUTIFULLY :)

Oh, one more thing, I really, really love the changes you made in the current release with the cleanup of the minified code base.

However, the require('fs') line makes React Native crash due to it not supporting dynamic requires (Probably due to sad, probably security-related reasons)
This particular line (10056 in indexeddbshim-noninvasive)
var fs = {}.toString.call(process) === '[object process]' ? require('fs') : null

Right now I've changed it to this
var fsreq = {}.toString.call(process) === '[object process]' ? 'fs' : null; if(fsreq) { var fs = require(fsreq); }
And it works.

Sorry for the long post! I'm really excited to have solved this problem, and I'm very, very thankful to you for maintaining this library, and answering to issues with edge cases like this :)

@brettz9
Copy link
Collaborator

brettz9 commented Feb 24, 2020

Great work finding the problem! I plan to look into a fix for that, and for the Rollup change (how require('fs') was added), as I may have some time.

Thank you for your persistence in helping us coming to a resolution.

I guess it was not an underspecification within SQLite but that Android just has a bug in following this from the SQlite docs: "Boolean values are stored as integers 0 (false) and 1 (true)."

Btw, would you mind testing to see whether we even need to change the BOOLEAN type in the CREATE TABLE syntax? It sounds like SQLite is documented as converting this automatically to NUMERIC. While I wouldn't expect making the type explicitly NUMERIC to be a breaking change for other environments if Android requires it, I'd rather not take the risk if it is not necessary.

Btw, it looks like from the issue you describe in requiring "fs" that you mean the problem with React Native is in handling static (fs) requires rather than dynamic ones. If I'm understanding it correctly, this comment speaks to your workaround as well.

@swittk
Copy link

swittk commented Feb 25, 2020

Hi. I've just verified that changing from BOOLEAN to NUMERIC is not necessary; just the Number() cast is enough to retain the correct data.

@swittk
Copy link

swittk commented Feb 25, 2020

Also, I tried the latest committed code. Using constant strings causes react native to complain too, so the lines
const fsStr = 'fs';
// eslint-disable-next-line no-undef, import/no-dynamic-require
const fs = ({}.toString.call(process) === '[object process]') ? require(fsStr) : null;
throw the same error as before.

Currently I've converted it to this to work.
const fsStr = ({}.toString.call(process) === '[object process]') ? 'fs' : null;
const fs = fsStr ? require(fsStr) : null;

@swittk
Copy link

swittk commented Mar 3, 2020

Seems the version on npm isn't updating. I noticed something with the TravisCI build failing?

@brettz9
Copy link
Collaborator

brettz9 commented Mar 6, 2020

Apologies, need a little time to do a final review.

@brettz9
Copy link
Collaborator

brettz9 commented Mar 17, 2020

See #352 (comment) . In short, if folks could test master on https://github.com/brettz9/IndexedDBShim , I'd like to see if that fix will work in both React Native and Webpack, etc. I should then be able to look to a release.

@niklasnatter
Copy link

niklasnatter commented Jun 26, 2020

Hey,
I just wanted to thank you for your work on this!

I am using the firebase-js-sdk in my expo app and figured that offline persistence is not working because IndexedDB is not available in the react-native JS environment (firebase/firebase-js-sdk#436). Thanks to your changes, I managed to provide IndexedDB to the firebase-js-sdk with the following lines:

import * as SQLite from 'expo-sqlite';
import setGlobalVars from 'indexeddbshim/dist/indexeddbshim-noninvasive';

setGlobalVars(window, { checkOrigin: false, win: SQLite });

@SohelIslamImran
Copy link

Is there any way to use Indexed DB Polyfill in React Native Expo?
I'm getting this error while using https://gist.github.com/zwily/e9e97e0f9f523a72c24c7df01d889482

 WARN  [2022-11-14T20:48:04.371Z]  @firebase/app: Firebase: Error thrown when reading from IndexedDB. Original error: undefined is not a function (near '...(0, _idb.openDB)...'). (app/idb-get).
 WARN  [2022-11-14T20:48:04.402Z]  @firebase/app: Firebase: Error thrown when reading from IndexedDB. Original error: undefined is not a function (near '...(0, _idb.openDB)...'). (app/idb-get).
 WARN  [2022-11-14T20:48:04.415Z]  @firebase/app: Firebase: Error thrown when writing to IndexedDB. Original error: undefined is not a function (near '...(0, _idb.openDB)...'). (app/idb-set).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants