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

Promise totally rewritten + Cleaner Dexie code #242

Merged
merged 29 commits into from May 3, 2016
Merged

Promise totally rewritten + Cleaner Dexie code #242

merged 29 commits into from May 3, 2016

Conversation

dfahlander
Copy link
Collaborator

The result of some weeks of refactoring.
The main issue was that Dexie.Promise were:

  1. Executing then-handlers in long call stacks making it hard to debug when error occur
  2. Uncaught rejections where not possible to detect if direct exceptions had occurred in the constructor of a Promise.
  3. Dexie needed to adjust to these limitations of the Promise class - to manually catch direct exceptions and abort the transaction directly - which made large amounts of the Dexie code mysterious and hard to understand the reason behind.

I also took the chance to simplify lots of code in Dexie, like db.open() and runUpgraders(). Now you don't have to step on your toes when dealing with promises anymore.

Optimized Promise class as much as possible and made it support long asyncronic stacks when in debug mode (which is now the default mode when served from localhost. Can be changed using Dexie.debug = false / Dexie.debug = true).

…iled transactionless operations.

This unit test confirms that.
Happens in Firefox when trying to open two different DBs simultanously (BUG, Firefox!)
Think it may happen when using IndexedDBShim as well.
Passes tests-promise.js but fails many other tests because Dexie needs to be adjusted because this version's reject callback does not return a boolean wether it was caught or not.
…d mippled with and documented the source.

Dexie.js is NOT WORKING STATE.
Works well in promise-tests but not yet adjusted Dexie for the new Promise class.
Also, not yet tested the new interopability with standard promises.
Dexie.js fails completely still. (Not any unit test succeeds except Promise tests).
…tests working.

Problems in some unit tests in tests-exception-handling and tests-transaction.
Look into that tonight or tomorrow.
…ise.usePSD could be removed. Instead, making sure to use wrap() where needed.

Promise.newPSD() replaced with newScope(). Some occurrancies of newPSD() could be removed because they are now handled through wrap().
Making sure iterate() will always wrap() the value callback function. But not the filter function.
Changed the way hooks and their hookContext are used.
All CRUD-hook tests pass.
Still failing as in previous checkin, on transaction and exception-handling tests.
BUT NOw we have a real problem: transaction unit tests fails when ran after each other but succeeds when running one by one. ?!!
Tested on Firefox, IE11 and Chrome.
Haven't tested the addons yet.
…ad but for people having old addons, it's better that they're not forced to update them just because dexie is updated.
Fixes:
* Proper stacks on all exceptions
* transaction cancelled errors due to transaction-specific scheduler. Removed that and begin each transaction scope with Promise.resolve() instead.
* Handle DatabaseClosedError gracefully in Dexie.Observable.js.
* Fix Dexie.ignoreTransaction().
* Support for iterables in Promise.all() and Promise.race()
* Support for iterables in WhereClause methods that takes arrays, such as anyOf()
* No leaking arguments (https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#32-leaking-arguments)
* Removed much of false positives unhandled rejection logs. Still some remaining when exception is thrown in populate event.
* Bugfix and new unit test: Calling db.open() a second time would always resolve even if an error occurred.
…stack support or not.

Now, default to false even though run from localhost. Reason: Tests run some seconds faster without it and if CI scripts fails, we could repeat the tests with long stacks turned on.
Still, debug mode will be the default when apps are run from localhost in production. This is because people wont read about the posibility to turn it on or off and it's better to default
to something that helps out finding errors. Also, it's only on localhost this is turned on.
The most important feature with this checkin is the following:

Before, you could not save a Collection and reuse it in another transaction because each Collection and Table where bound to the transaction where they were retrieved. Now, the transaction is looked up when the operation happens instead.

The optimization part of this is that new Transaction() will not have to create new Tables. Instead, there is only one set of Tables. Always the same in transaction and on database.

There are failing unit tests when running with Dexie.Syncable turned on. Those errors occurred in an earlier checkin. Need to find out why.
…ents when using shorhand of subscribe function.
…cable and observable addons.

It's just one unit test specifically for Dexie.Syncable that fails on the last assetion, but that has been the state foreverish.
@dfahlander dfahlander merged commit 6309c87 into master May 3, 2016
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.

None yet

1 participant