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

Native async/await and upgrade handlers #612

Closed
sechel opened this issue Nov 9, 2017 · 4 comments
Closed

Native async/await and upgrade handlers #612

sechel opened this issue Nov 9, 2017 · 4 comments
Labels

Comments

@sechel
Copy link

sechel commented Nov 9, 2017

Hi, I'm currently trying to upgrade my codebase to native es2017 and especially native async/await. I am aware that I will have to to do some extra work to include Firefox. This is the story about Chrome. I'm using Dexie 2.0.1 and everything seems to work well so far. The only thing that does not work is migration with async callbacks. I prepared a JSFiddle to illustrate this (This contains also a version with pure then callbacks and a version involving generator functions both of witch work fine): https://fiddle.jshell.net/sechel/06gcbbpL/

let db = new Dexie(dbName);
db.version(1).stores({test: '++id'});
let data = {id: 1, value: 1};
await db.test.put(data);
db.close();
let migrated = new Dexie(dbName);
migrated.version(1).stores({test: '++id'})
migrated.version(2).stores({test: '++id, value'}).upgrade(async t => {
console.log('2 start');
await t.test.toCollection().modify(d => d.value += 1);
console.log('2 mid');
await t.test.toCollection().modify(d => d.value += 1);
console.log('2 end');
});
migrated.version(3).stores({test: '++id, value'}).upgrade(async t => {
console.log('3 start');
await t.test.toCollection().modify(d => d.value *= 2);
console.log('3 mid');
await t.test.toCollection().modify(d => d.value *= 2);    
console.log('3 end');    
});  
let dataExpected = {id: 1, value: 12};
let dataMigrated = await migrated.test.get(1);
assert.deepEqual(dataMigrated, dataExpected, 'migration successfully performed');
await migrated.delete();

The console output illustrates that there is something strange going on.

2 start
3 start
2 mid
3 mid
2 end
3 end

It seems to me as if the async handlers are not called sequentially but somehow in parallel on the same transaction!? Does that make sense, am I missing something?

@dfahlander
Copy link
Collaborator

Ok, I think the upgraders fail to reference count the db operations as standard promises take place instead of Dexie.Promise. Reference counting is not needed with async functions and we should really respect the returned promise instead. Could be easily fixed.

@dfahlander dfahlander added the bug label Nov 9, 2017
@sechel
Copy link
Author

sechel commented Nov 10, 2017

Hi David, thanks for the reply, a fix would be great.

@dfahlander
Copy link
Collaborator

I'll try fix it next week. Just a note for other readers : using native (non-transpiled) async await can only work well on recent versions of Chrome, Edge, opera and Safari. Firefox still has issues when involving native promises in an indexedDB transaction, and not all users have the latest versions of their browsers. Therefore, it is still recommended to transpile async/await code using either babel or typescript if it should work on most end users browsers.

dfahlander added a commit that referenced this issue Nov 12, 2018
dfahlander added a commit that referenced this issue Nov 12, 2018
Calling upgrader the same way as it is done in transaction scope
dfahlander added a commit that referenced this issue Nov 12, 2018
Calling upgrader the same way as it is done in transaction scope
dfahlander added a commit that referenced this issue Nov 12, 2018
dfahlander added a commit that referenced this issue Nov 12, 2018
Calling upgrader the same way as it is done in transaction scope
@dfahlander
Copy link
Collaborator

Closing as it is now resolved in master branch and will be part of next 3.0.0-release.

dfahlander added a commit that referenced this issue Nov 12, 2018
Repro of #770. Once I cherry picked the resolution of #612, it also fixed #770 as it does not wait for Promise.follow() to complete when upgrader returns a Promise.

This is in line with how db.transaction() works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants