fix cursor error in Safari #420
Conversation
src/methods/indexed-db.js
Outdated
| const keyRangeValue = IDBKeyRange.bound(lastCursorId + 1, Infinity); | ||
| return new Promise(res => { | ||
| objectStore.openCursor(keyRangeValue).onsuccess = ev => { | ||
| objectStore.openCursor(lastCursorId + 1).onsuccess = ev => { |
There was a problem hiding this comment.
This is the one change.
|
I migrated the CI to github actions now. |
|
I rebased and ran the tests. I couldn't get the browser tests to pass on my branch, but they failed the same way on master. I also had a ton of changes to the dist folder, so I suspect that the build runs differently on my system. |
|
Kudos to your tests, there was a bug in my code. I have fixed it. The localStorage tests still fail on my machine, but they work in CI 🤷 |
| } else { | ||
| ret.push(cursor.value); | ||
| cursor.continue(); | ||
| } |
There was a problem hiding this comment.
This is the change. It opens a cursor on the store without a KeyRange, then advances the cursor. This is because creating the key range can sometimes error in Safari.
|
Thanks for fixing. |
|
Performance testing is always a good idea. This avoids iterating over every message, but it does always pull the first item out. By advancing the cursor to the desired ID, it skips anything between the first message and the desired message. I like the idea of only doing it when the KeyRange fails, that's very clever! Another solution that would improve speed is to add an empty message with an ID of 1, then never delete it. I can add the KeyRange idea on Tuesday. |
|
Ok, I pushed the change. |
|
Thank you very much. Merged. Release will come next week. |
|
No problem! Thanks for the great library! |
|
It seemed that it's not working in iframe in safari 13.0.5 |
This change is one line and a spelling correction, I have created a comment in the files section. Please advise on the dist files that are significantly changed.
The app I work on has seven broadcast channels and a large application database. Some of my channels were consistently getting this error, then not working:
Interestingly, it was always the same databases.
I don't know the true root cause, but I did find a fix. For some reason, Safari was not happy with the key range, and it would always error trying to create the cursor. Fortunately, this library does not require the range and the cursor can be passed the last known index.