Open arolson101 opened 6 years ago
nponiros
commented
6 years ago Are you using the latest versions of dexie and dexie-observable and dexie-syncable (in case you are using it)? I don't remember having an issue in the browser and I think I had fixed dexie-syncable so the save method is not written into the db.
arolson101
commented
6 years ago these are the versions I'm using:
"dexie": "^2.0.1",
"dexie-observable": "^1.0.0-beta.4",
"dexie-syncable": "^1.0.0-beta.4",
"fake-indexeddb": "^2.0.3",I believe this is an issue with realistic-structured-clone. It says in the source that the plain object check is too restrictive. Assuming that syncable works in multiple browsers, then I would say the issue is with fake-indexeddb.
Edit: reading a bit more through the code, I have no real clue what is going on..
Can you check in the browser if the sync nodes have a save method when saved in the db?
arolson101
commented
6 years ago using Chrome the _syncNodes db has an object that does not have a save key of any type.
so my guess is that the issue is with realistic-structured-clone
arolson101
commented
6 years ago Although Chrome allows it, I'm not convinced that the spec does. Would it be possible to clone the sync node to a new object, and save that instead?
nponiros
commented
6 years ago We are saving this which I would assume doesn't contain the save method. It also worked in safari for me so not just chrome. I believe our code is correct but you never know. I would have to debug fake-indexeddb to know where exactly the exception is thrown and compare with our code/the spec to know where the issue is.
We could also copy the object but I would rather fix the real issue instead of just implementing something.
You could try to define your own save method at least for the tests. Have a look at https://github.com/dfahlander/Dexie.js/blob/master/addons/Dexie.Syncable/src/Dexie.Syncable.js#L225 I think saving Object.assing ({}, this) instead of just this would do the trick.
arolson101
commented
6 years ago The code in question is not the Dexie.Syncable save method (although that might be a problem, too), but Dexie.Observable's db.on("ready") handler startObserving function saving mySyncNode.node here
}).then(()=>{
// Add our node to DB and start subscribing to events
return db._syncNodes.add(/*>>>>>*/mySyncNode.node /*<<<<<*/).then(function() {
Observable.on('latestRevisionIncremented', onLatestRevisionIncremented); // Wakeup when a new revision is available.
Observable.on('beforeunload', onBeforeUnload);
Observable.on('suicideNurseCall', onSuicide);
Observable.on('intercomm', onIntercomm);
// Start polling for changes and do cleanups:
pollHandle = setTimeout(poll, LOCAL_POLL);
// Start heartbeat
heartbeatHandle = setTimeout(heartbeat, HEARTBEAT_INTERVAL);
});Hm okay, just got caught up in dexie-syncable because that is the code actually defining the save method of the SyncNode class which seems to be causing the issue. I still have no clue what exactly causes the failure in the data cloning algorithm. Is the isPlainObject test really failing because of the save method or is it failing because mySyncNode.node is an instance of SyncNode and in that sense not a plain object with Object as direct prototype?
From the isPlainObject docs:
Checks if value is a plain object, that is, an object created by the Object constructor or one with a [[Prototype]] of null.From that I would assume the check fails because we are using a class. From my understanding of the spec, what we are doing in dexie-observable is allowed, which would mean the issue is with fake-indexeddb.
Cloning the SyncNode object and saving that should be possible but assuming that we are doing everything correct currently, I don't feel comfortable deciding this change. I would rather have @dfahlander have a look at this and let us know if cloning would work just as good.
dfahlander
commented
6 years ago There is nothing in the structured clone algorithm that forbids objects with __proto pointing to other than Object.prototype to be cloned (https://www.w3.org/TR/html52/single-page.html#structuredserializeforstorage). Normal objects should be clonable but their prototype chain will not be preserved (https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm).
However, putting a function property directly on the object should not be supported. But in this case, the save() method lies in the prototype.
dfahlander
commented
6 years ago The issue will persist for anyone storing their classes into Dexie. According to IndexedDB specification, refering to HTML5 spec structuredserializeforstorage, refering to structuredserialize_internal, this should be possible.
If we do a fix in Dexie.Observable, people storing classes in their app code will still suffer. So the fix should instead be done within realistic-structured-clone somewhere at this location by allowing objects with custom prototypes.
An alternative would also be for fake-indexedDB to replace realistic-structured-clone with typeson, which is developed by me and @brettz9 and used in IndexedDBShim for the same purpose. If so, use the structured-cloning-throwing preset from typeson-registry
stevenxxiu
commented
5 years ago Any updates on this and any workarounds? Just ran into this issue on node with latest fake-IndexedDB 2.1.1.
None of my objects have a save method.
My error looks like:
DataCloneError: The data being stored could not be cloned by the internal structured cloning algorithm.
at new DataCloneError (node_modules/fake-indexeddb/build/lib/errors.js:57:28)
at Object.structuredClone [as default] (node_modules/fake-indexeddb/build/lib/structuredClone.js:10:15)
at buildRecordAddPut (node_modules/fake-indexeddb/build/FDBObjectStore.js:36:42)
at FDBObjectStore.Object.<anonymous>.FDBObjectStore.put (node_modules/fake-indexeddb/build/FDBObjectStore.js:130:22)
at node_modules/dexie/src/Dexie.js:1324:85
at supplyIdbStore (node_modules/dexie/src/Dexie.js:935:24)
at node_modules/dexie/src/Dexie.js:427:28
at usePSD (node_modules/dexie/src/Promise.js:782:16)
at newScope (node_modules/dexie/src/Promise.js:670:14)
at node_modules/dexie/src/Dexie.js:425:24
at node_modules/dexie/src/Dexie.js:1536:30
at executePromiseTask (node_modules/dexie/src/Promise.js:373:9)
at new Promise (node_modules/dexie/src/Promise.js:172:5)
at Transaction._promise (node_modules/dexie/src/Dexie.js:1535:25)
at tempTransaction (node_modules/dexie/src/Dexie.js:424:26)
at node_modules/dexie/src/Dexie.js:420:44
at node_modules/dexie/src/Promise.js:801:23
at callListener (node_modules/dexie/src/Promise.js:469:19)
at endMicroTickScope (node_modules/dexie/src/Promise.js:555:25)
at FDBOpenDBRequest.onsuccess (node_modules/dexie/src/Promise.js:624:30)
at invokeEventListeners (node_modules/fake-indexeddb/build/lib/FakeEventTarget.js:66:31)
at FDBOpenDBRequest.Object.<anonymous>.FakeEventTarget.dispatchEvent (node_modules/fake-indexeddb/build/lib/FakeEventTarget.js:119:13)
at node_modules/fake-indexeddb/build/FDBFactory.js:288:25
at node_modules/fake-indexeddb/build/FDBFactory.js:211:13
at Immediate.<anonymous> (node_modules/fake-indexeddb/build/FDBFactory.js:183:21)
From previous:
at Transaction._promise (node_modules/dexie/src/Dexie.js:1535:25)
at tempTransaction (node_modules/dexie/src/Dexie.js:424:26)
at node_modules/dexie/src/Dexie.js:420:44
at node_modules/dexie/src/Promise.js:801:23
at callListener (node_modules/dexie/src/Promise.js:469:19)
at endMicroTickScope (node_modules/dexie/src/Promise.js:555:25)
at FDBOpenDBRequest.onsuccess (node_modules/dexie/src/Promise.js:624:30)
at invokeEventListeners (node_modules/fake-indexeddb/build/lib/FakeEventTarget.js:66:31)
at FDBOpenDBRequest.Object.<anonymous>.FakeEventTarget.dispatchEvent (node_modules/fake-indexeddb/build/lib/FakeEventTarget.js:119:13)
at node_modules/fake-indexeddb/build/FDBFactory.js:288:25
at node_modules/fake-indexeddb/build/FDBFactory.js:211:13
at Immediate.<anonymous> (node_modules/fake-indexeddb/build/FDBFactory.js:183:21)As of my understanding, fixes have been made in both fake-indexeddb and typeson-registry. I think we should close this original issue now, is that right, @dumbmatter / @brettz9? If so, I think @stevenxxiu could have other reason than this original issue.
dumbmatter
commented
5 years ago Yeah, fake-indexeddb now uses typeson, so this is either a legitimately invalid object (containing a function or something) or a bug in typeson. I'd guess the former is more likely, but who knows :)
brettz9
commented
5 years ago Yes, we'd need more info on the triggering object. There could also be some built-in type that is potentially serializable but for which we have no encapsulator in the registry.
stevenxxiu
commented
5 years ago Ahh the issue is due to Blob. Without Blob it works. Is there a way to get it working with Blob? (It's defined at node_modules/jest-cli/node_modules/jsdom/lib/jsdom/living/generated/Blob.js) Here is some minimal code that reproduces it:
import Dexie from 'dexie'
import indexedDB from 'fake-indexeddb'
import FDBKeyRange from 'fake-indexeddb/lib/FDBKeyRange'
describe('DeflateWriter', () => {
test('dummy', async () => {
const db = new Dexie('recordings', {indexedDB: indexedDB, IDBKeyRange: FDBKeyRange})
db.version(1).stores({files: 'name'})
// this works
await db.files.put({name: 'file.bin', data: [new Uint8Array([1, 2, 3])]})
// this doesn't
await db.files.put({name: 'file.bin', data: new Blob([new Uint8Array([1, 2, 3])])})
})
})It appears to me that the issue may be that fake-indexeddb, in relying on realistic-structured-clone, which in turn relies on an earlier version of typeson-registry (and typeson), only supports string Blobs (it depends on v1.0.0-alpha.20 whereas it was in subsequent versions of typeson-registry that array buffer support was added for Blobs).
I would see about patching fake-indexeddb/realistic-structured-clone and see if that solves it...
brettz9
commented
5 years ago I would also be sure to use the latest typeson-registry (and not just an incremented version), as there were successive fixes/improvements for Blob handling.
dumbmatter
commented
5 years ago It already uses the latest typeson-registry, version 1.0.0-alpha.28.
The problem seems to be that typeson-registry assumes the presence of various web APIs when processing Blobs. Like in this code:
const jsdom = require("jsdom");
const Typeson = require("typeson");
const structuredCloningThrowing = require("typeson-registry/dist/presets/structured-cloning-throwing");
const { JSDOM } = jsdom;
const { window } = new JSDOM("", {
url: "https://example.com/",
});
const data = new window.Blob([new Uint8Array([1, 2, 3])]);
console.log(data);
const TSON = new Typeson().register(structuredCloningThrowing);
const encapsulated = TSON.encapsulate(data);
console.log(encapsulated);You get:
ReferenceError: XMLHttpRequest is not definedIf you put all the jsdom stuff in global scope with something like Object.assign(global, window);, then the error changes to:
TypeError: URL.createObjectURL is not a functionI didn't go further down the rabbit hole of polyfilling web APIs, but maybe it's possible to make it work. It'd be nice if it worked more easily, though. I don't know enough about Blobs to say if the problem lies in typeson-registry or jsdom, or if there's just no good solution. It's definitely not Dexie's fault though :)
brettz9
commented
5 years ago @dumbmatter , in looking at https://github.com/dumbmatter/realistic-structured-clone/blob/master/package.json#L32-L33 (in master here, but also on the typeson branch and latest 2.0.2 tag), the versions are outdated. I haven't checked what is actually on npm, but I assume you'll want to ensure if you have an updated version, that it is pushed to Github.
dumbmatter
commented
5 years ago The ^ means it gets versions 5.13.0 and 1.0.0-alpha.28 if you install it today.
brettz9
commented
5 years ago Oops, I knew that. :) (Sorry, am too used to using npm-check-updates and viewing versions with a mind to updating them as needed regardless of meeting semver requirements.)
If @stevenxxiu is actually using the latest versions, including the dependent typeson/typeson-registry, it sounds like the issue is being reported by Safari/iOS and somehow typeson is attempting to clone (i.e., something did not get serialized as it should have). But the latest typeson has no problem with the structure given, so I'm thinking he's probably using an outdated version.
stevenxxiu
commented
5 years ago I think I'm using latest versions, just did a npm-check-updates, deleted my node_modules and reinstalled. I can reproduce @dumbmatter 's issue, and my own code has the DataCloneError: The data being stored could not be cloned by the internal structured cloning algorithm. still.
The error has an issue at https://github.com/jsdom/jsdom/issues/1721. I tried but couldn't fix it.
brettz9
commented
5 years ago If there is an issue in typeson-registry, it should be reproducible in typeson-registry. (You can confirm the versions you are using, @stevenxxiu , by looking in package-lock.json or looking in the specific package.json of the package in question within node_modules.)
brettz9
commented
5 years ago As far as URL.createObjectURL is not a function, a polyfill is provided in typeson-registry/polyfills (it only polyfills partially and for the sake of Node + jsdom, but it does the job needed for Blob and File).
stevenxxiu
commented
5 years ago Tried that just now. For some reason typeson wants to retrieve the blob over a XMLHttpRequest:
replace (b) { // Sync
const req = new XMLHttpRequest();
req.overrideMimeType('text/plain; charset=x-user-defined');
// eslint-disable-next-line max-len
// eslint-disable-next-line node/no-unsupported-features/node-builtins
req.open('GET', URL.createObjectURL(b), false); // Sync
if (req.status !== 200 && req.status !== 0) {
throw new Error('Bad Blob access: ' + req.status);
}
req.send();
return {
type: b.type,
stringContents: req.responseText
};
},Yes, using XMLHttpRequest gives a backward-compatible way of getting binary data in the browser: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/Sending_and_Receiving_Binary_Data#Receiving_binary_data_in_older_browsers .
From Node, you should be able to define a global XMLHttpRequest by using the XMLHttpRequest object from a jsdom instance.
The createObjectURL polyfill has two methods you should use, createObjectURL (which you can assign to a global URL (an object you can also get from jsdom if not Node itself) and xmlHttpRequestOverrideMimeType which should be called to override XMLHttpRequest.prototype.overrideMimeType. You can see this being done in https://github.com/dfahlander/typeson-registry/blob/master/test/test-environment.js#L7-L18
stevenxxiu
commented
5 years ago Thanks so much, that got it working!
I would expect getting this to work in jest would take some more effort, haven't got it working yet.
stevenxxiu
commented
4 years ago Almost a year later I've come back to this. I still couldn't get it to work in Jest after trying for quite a while now.
The simplifed version of the test I'm trying ot run in jest looks like:
import Dexie from 'dexie'
import IndexedDB from 'fake-indexeddb'
import FDBKeyRange from 'fake-indexeddb/lib/FDBKeyRange'
test('test', async () => {
const db = new Dexie('recordings', {indexedDB: IndexedDB, IDBKeyRange: FDBKeyRange})
db.version(1).stores({files: 'name'})
const table = db.files
const blob = new Blob([new Uint8Array([1, 2, 3])])
await table.put({name: 'file.bin', data: blob})
})I got the following working in node however, but not sure how to adapt it to jest:
const jsdom = require('jsdom')
const Typeson = require('typeson')
const structuredCloningThrowing = require('typeson-registry/dist/presets/structured-cloning-throwing')
const { JSDOM } = jsdom
const { window } = new JSDOM('', {
url: 'https://example.com/',
})
Object.assign(global, window)
global.XMLHttpRequest = window.XMLHttpRequest
const { createObjectURL, xmlHttpRequestOverrideMimeType} = require('typeson-registry/polyfills/createObjectURL-cjs.js')
URL.createObjectURL = createObjectURL
XMLHttpRequest.prototype.overrideMimeType = xmlHttpRequestOverrideMimeType()
const data = new window.Blob([new Uint8Array([1, 2, 3])])
console.log(data)
const TSON = new Typeson().register(structuredCloningThrowing)
const encapsulated = TSON.encapsulate(data)
console.log(encapsulated)In jest I get TypeError: Cannot read property '_buffer' of undefined for the line const encapsulated = TSON.encapsulate(data).
@brettz9 Any pointers? Much appreciated.
brettz9
commented
4 years ago @stevenxxiu : _buffer is used in typeson-registry's /polyfills/createObjectURL.js to dig into jsdom's implementation of the Blob to get at its contents. Be sure you are using jsdom to build the Blob (as in your Node example where it uses window.Blob). If that doesn't work, try logging within 'typeson-registry/polyfills/createObjectURL-cjs.js', whether when URL.createObjectURL is called or when the monkey-patched XMLHttpRequest.prototype.open is called (see xmlHttpRequestOverrideMimeType).
stevenxxiu
commented
4 years ago My code is now the following:
import Dexie from 'dexie'
import IndexedDB from 'fake-indexeddb'
import FDBKeyRange from 'fake-indexeddb/lib/FDBKeyRange'
import { JSDOM } from 'jsdom'
import { createObjectURL, xmlHttpRequestOverrideMimeType} from 'typeson-registry/polyfills/createObjectURL-cjs.js'
test('test', async () => {
const { window } = new JSDOM('', {
url: 'https://example.com/',
})
global.XMLHttpRequest = window.XMLHttpRequest
URL.createObjectURL = createObjectURL
XMLHttpRequest.prototype.overrideMimeType = xmlHttpRequestOverrideMimeType()
const db = new Dexie('recordings', {indexedDB: IndexedDB, IDBKeyRange: FDBKeyRange})
db.version(1).stores({files: 'name'})
const table = db.files
const blob = new window.Blob([new Uint8Array([1, 2, 3])])
await table.put({name: 'file.bin', data: blob})
})URL.createObjectURL is called, but I'm not sure why xmlHttpRequestOverrideMimeType's returned function is not.
brettz9
commented
4 years ago That function is used inside typeson-registry' /types/blob.js to serialize Blobs. I don't really know how Dexie works things, but I'm guessing the problem is that fake-indexeddb is not using typeson-registry and you actually need IndexedDBShim as it internally uses typeson-registry when cloning (those polyfills help Node work with typeson-registry's structured cloning preset (which includes the Blob serialization)).
When running in Node, typeson-registry needs the polyfills you have added for createObjectURL and overrideMimeType, but it will also need to be able to use them to serialize the Blob to be able to save it. If you use indexedDBShim, it will do this.
stevenxxiu
commented
4 years ago Ok I tried IndexedDBShim with the code:
import Dexie from 'dexie'
import setGlobalVars from 'indexeddbshim'
import { JSDOM } from 'jsdom'
import { createObjectURL, xmlHttpRequestOverrideMimeType} from 'typeson-registry/polyfills/createObjectURL-cjs.js'
test('test', async () => {
const { window } = new JSDOM('', {
url: 'https://example.com/',
})
setGlobalVars(window)
global.XMLHttpRequest = window.XMLHttpRequest
URL.createObjectURL = createObjectURL
XMLHttpRequest.prototype.overrideMimeType = xmlHttpRequestOverrideMimeType()
const db = new Dexie('recordings', {indexedDB: window.shimIndexedDB, IDBKeyRange: window.IDBKeyRange})
db.version(1).stores({files: 'name'})
const table = db.files
const blob = new window.Blob([new Uint8Array([1, 2, 3])])
await table.put({name: 'file.bin', data: blob})
})I'm getting the error:
TypeError: Illegal invocation
at XMLHttpRequest.call (xxx/.node_modules/lib/node_modules/jest/node_modules/jsdom/lib/jsdom/living/generated/XMLHttpRequest.js:78:15)
at XMLHttpRequest.open (xxx/node_modules/typeson-registry/polyfills/createObjectURL.js:88:33)Not really sure how to proceed.
brettz9
commented
4 years ago Try pegging jsdom for now to 16.1.0 (the last tested version for typeson-registry). It seems you may be encountering a new restriction (would be nice if jsdom itself would implement https://github.com/jsdom/jsdom/issues/1721 including the XMLHttpRequest support so the polyfills wouldn't be necessary).
For further discussion on this issue though, why don't you file an issue at https://github.com/indexeddbshim/indexeddbshim so we can discuss there, as this isn't really related to this issue anymore.
(Also, note that the package is @indexeddbshim/indexeddbshim--the current replacement for the fact that the owner of indexeddbshim hadn't been in touch when I wanted to update the repo settings, so @indexeddbshim/indexeddbshim is a more recently maintained fork (though in this case, it probably won't be important).)
I'm running under node (for unit testing) with
fake-indexeddbanddexie-observableI get a'DataCloneError'("The data being stored could not be cloned by the internal structured cloning algorithm.") because instartObserving(Dexie.Observable.js:255 --db._syncNodes.add(mySyncNode.node)) the object being saved has a 'save' method and failsisPlainObjectinrealistic-structured-clone: