feat(defineDriver): set up custom drivers syncronously if there's no _support property or it's not a function #692

gabrielmaldi · 2017-05-02T08:00:42Z

This addresses #686 according to #686 (comment).

_support property or it's not a function. Closes localForage#686 Closes localForage#692

gabrielmaldi · 2017-05-02T08:23:16Z

dist/localforage.js

@@ -343,7 +343,7 @@ if (typeof global.Promise !== 'function') {
 },{"2":2}],4:[function(_dereq_,module,exports){
 'use strict';

-var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol ? "symbol" : typeof obj; };
+var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; };


I don't know why this line is changing. Should this be part of the commit?

If you just did a fresh npm install then don't worry about it.
It would be great if the dist files were in a different commit, but it's not mandatory.

Yeah, that's it. I separated the code into two commits 👍

thgreasi · 2017-05-02T09:41:09Z

I think that the code should be moved outside the new Promise(function(){...}), since according to the spec you can't bet that the constructor function runs synchronously.

gabrielmaldi · 2017-05-02T10:19:59Z

I think it's guaranteed that the constructor will run immediately:

From MDN (emphasis mine):

new Promise( /* executor */ function(resolve, reject) { ... } );
executor
A function that is passed with the arguments resolve and reject. The executor function is executed immediately by the Promise implementation, passing resolve and reject functions (the executor is called before the Promise constructor even returns the created object). The resolve and reject functions, when called, resolve or reject the promise, respectively. The executor normally initiates some asynchronous work, and then, once that completes, either calls the resolve function to resolve the promise or else rejects it if an error occurred.
If an error is thrown in the executor function, the promise is rejected. The return value of the executor is ignored.

From promisejs.org (emphasis also mine):

We use new Promise to construct the promise. We give the constructor a factory function which does the actual work. This function is called immediately with two arguments. The first argument fulfills the promise and the second argument rejects the promise. Once the operation has completed, we call the appropriate function.

However, if you still feel it's safer to move the code outside the constructor, I'll make the change.

thgreasi · 2017-05-02T10:35:43Z

My bad 😞
It seems I was a bit confused with an other part of the Promises spec.
If the current code works for you then I;m fine with it.
It would be great to have a test case as well, but if it troubles you then I can give it a try during the weekend,
Kudos for the PR.

_support property or it's not a function. Closes localForage#686 Closes localForage#692

_support property or it's not a function.

thgreasi · 2017-05-02T18:42:11Z

Thanks for this!!

gabrielmaldi · 2017-05-02T18:51:42Z

Thanks for the tests, I had taken a look and was planning on doing something very similar to what you did. 👍

gabrielmaldi force-pushed the define-driver-sync branch from 44dbfe1 to 031edbb Compare May 2, 2017 08:01

gabrielmaldi pushed a commit to gabrielmaldi/localForage that referenced this pull request May 2, 2017

feat(defineDriver): set up custom drivers syncronously if there's no

031edbb

_support property or it's not a function. Closes localForage#686 Closes localForage#692

gabrielmaldi force-pushed the define-driver-sync branch from 031edbb to 0225cee Compare May 2, 2017 08:08

gabrielmaldi pushed a commit to gabrielmaldi/localForage that referenced this pull request May 2, 2017

feat(defineDriver): set up custom drivers syncronously if there's no

0225cee

_support property or it's not a function. Closes localForage#686 Closes localForage#692

gabrielmaldi force-pushed the define-driver-sync branch from 0225cee to 34f1e7a Compare May 2, 2017 08:14

gabrielmaldi pushed a commit to gabrielmaldi/localForage that referenced this pull request May 2, 2017

feat(defineDriver): set up custom drivers syncronously if there's no

34f1e7a

_support property or it's not a function. Closes localForage#686 Closes localForage#692

gabrielmaldi force-pushed the define-driver-sync branch from 34f1e7a to 771fad6 Compare May 2, 2017 08:17

gabrielmaldi added a commit to gabrielmaldi/localForage that referenced this pull request May 2, 2017

feat(defineDriver): set up custom drivers syncronously if there's no

771fad6

_support property or it's not a function. Closes localForage#686 Closes localForage#692

gabrielmaldi commented May 2, 2017

View reviewed changes

This was referenced May 2, 2017

impossible to use as immediate fallback localForage/localForage-memoryStorageDriver#2

Closed

defineDriver's API introduces the need for ready event #686

Closed

Create memory.js for fallback #555

Closed

gabrielmaldi added 2 commits May 2, 2017 07:41

feat(defineDriver): set up custom drivers syncronously if there's no

3f7df54

_support property or it's not a function. Closes localForage#686 Closes localForage#692

chore(dist): set up custom drivers syncronously if there's no

c523f90

_support property or it's not a function.

gabrielmaldi force-pushed the define-driver-sync branch from 771fad6 to c523f90 Compare May 2, 2017 10:42

thgreasi mentioned this pull request May 2, 2017

feat(defineDriver): set up custom drivers syncronously if there's no _support property or it's not a function #693

Merged

thgreasi merged commit c523f90 into localForage:master May 2, 2017

gabrielmaldi deleted the define-driver-sync branch May 2, 2017 18:49

thgreasi mentioned this pull request Sep 24, 2017

feat: use custom driver API to define internal drivers #723

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(defineDriver): set up custom drivers syncronously if there's no _support property or it's not a function #692

feat(defineDriver): set up custom drivers syncronously if there's no _support property or it's not a function #692

gabrielmaldi commented May 2, 2017

gabrielmaldi May 2, 2017

thgreasi May 2, 2017

gabrielmaldi May 2, 2017

thgreasi commented May 2, 2017

gabrielmaldi commented May 2, 2017

thgreasi commented May 2, 2017 •

edited

thgreasi commented May 2, 2017

gabrielmaldi commented May 2, 2017

feat(defineDriver): set up custom drivers syncronously if there's no _support property or it's not a function #692

feat(defineDriver): set up custom drivers syncronously if there's no _support property or it's not a function #692

Conversation

gabrielmaldi commented May 2, 2017

gabrielmaldi May 2, 2017

Choose a reason for hiding this comment

thgreasi May 2, 2017

Choose a reason for hiding this comment

gabrielmaldi May 2, 2017

Choose a reason for hiding this comment

thgreasi commented May 2, 2017

gabrielmaldi commented May 2, 2017

thgreasi commented May 2, 2017 • edited

thgreasi commented May 2, 2017

gabrielmaldi commented May 2, 2017

thgreasi commented May 2, 2017 •

edited