I'm sympathic to having a dedicated Storage API and not just a File API :-)
void addEventListener(DOMString type, EventListener listener, optional boolean useCapture);
void removeEventListener(DOMString type, EventListener listener);
boolean dispatchEvent(DeviceStorageChangeEvent event);
=> This part can be removed by making DeviceStorage inherit from EventTarget
I see the idl file does it.
I don't understand the get/getEditable divide. getEditable is enough, no?
Both the enumerate and enumerateEditable could return ES6 generators [1] instead of a custom thing that mimics the exact same behavior. SpiderMonkey already has generators. If WedIDL has nothing to use ES generators, let's send feedback to get it.
About the DeviceStorageChangeEvent, what does path refer to? The notion of "absolute path" is hard to understand given that the Storage API is not a File API. If it was the absolute path of the underlying implementation, I think it'd be a security leak.
> Both the enumerate and enumerateEditable could return ES6 generators > [1] instead of a custom thing that mimics the exact same behavior. > SpiderMonkey already has generators. If WedIDL has nothing to use ES > generators, let's send feedback to get it.
For that part, it's been brought to my attention [1] that DeviceStorage is asynchronous, so an ES6 iterator (it's the interface. generators are an implementation of this interface and other things) isn't the right fit.
I guess I don't understand the point of the "continue". Apparently, there is an equivalent construct in IDB. I've seen code samples and the weird part is the .continue. Basically, the developer has to opt-in to continue enumerate results. It goes against the habit (and what's natural in my opinion) to continue by default and having an opt-in for *stopping* the enumeration:
* ES5 array extras enumerate all by default (you can throw to stop),
* iterators enumerate all by default (you have to throw a specific value to stop the iteration)
* for/for-in loops enumerate all by default and you have to "break" to stop the iteration
Is there a specific reason to have chosen the opposite direction for DeviceStorageCursor? Does the web dev wants to stop the enumeration by default?
If not, an "abort" function would be more relevant than a "continue" function.
An alternative is to return an array of DOMRequest (or promises), each corresponding to a given "file". With ES6 proxies (or C++ magic), everything, even the DOMRequests instances can be built lazily.
On Wed, Nov 7, 2012 at 3:33 AM, David Bruant <bruan...@gmail.com> wrote:
> Le 06/11/2012 18:36, David Bruant a écrit :
> Both the enumerate and enumerateEditable could return ES6 generators [1]
>> instead of a custom thing that mimics the exact same behavior. SpiderMonkey
>> already has generators. If WedIDL has nothing to use ES generators, let's
>> send feedback to get it.
> For that part, it's been brought to my attention [1] that DeviceStorage is
> asynchronous, so an ES6 iterator (it's the interface. generators are an
> implementation of this interface and other things) isn't the right fit.
> I guess I don't understand the point of the "continue". Apparently, there
> is an equivalent construct in IDB. I've seen code samples and the weird
> part is the .continue. Basically, the developer has to opt-in to continue
> enumerate results.
I think this is because you might need to do other asynchronous things
before getting the next result. Like:
storage = navigator.getDeviceStorage("images");
cursor = storage.enumerate(".");
cursor.onsuccess = function () {
var file = cursor.result;
req = new XMLHttpRequest();
req.open("POST", "/some-service");
req.onreadystatechange = function () {
if (req.readyState == 4) {
cursor.continue();
}
};
var reader = new FileReader();
reader.onload = function () {
req.send(reader.result);
};
reader.readAsArrayBuffer(file);
};
Once you start adding error handling and other stuff the logic could get
pretty complicated, and .continue() makes it more doable.
> On Wed, Nov 7, 2012 at 3:33 AM, David Bruant <bruan...@gmail.com > <mailto:bruan...@gmail.com>> wrote:
> Le 06/11/2012 18:36, David Bruant a écrit :
> Both the enumerate and enumerateEditable could return ES6
> generators [1] instead of a custom thing that mimics the exact
> same behavior. SpiderMonkey already has generators. If WedIDL
> has nothing to use ES generators, let's send feedback to get it.
> For that part, it's been brought to my attention [1] that
> DeviceStorage is asynchronous, so an ES6 iterator (it's the
> interface. generators are an implementation of this interface and
> other things) isn't the right fit.
> I guess I don't understand the point of the "continue".
> Apparently, there is an equivalent construct in IDB. I've seen
> code samples and the weird part is the .continue. Basically, the
> developer has to opt-in to continue enumerate results.
> I think this is because you might need to do other asynchronous things > before getting the next result. Like:
> storage = navigator.getDeviceStorage("images");
> cursor = storage.enumerate(".");
> cursor.onsuccess = function () {
> var file = cursor.result;
> req = new XMLHttpRequest();
> req.open("POST", "/some-service");
> req.onreadystatechange = function () {
> if (req.readyState == 4) {
> cursor.continue();
> }
> };
> var reader = new FileReader();
> reader.onload = function () {
> req.send(reader.result);
> };
> reader.readAsArrayBuffer(file);
> };
> Once you start adding error handling and other stuff the logic could > get pretty complicated, and .continue() makes it more doable.
I see. The problem with enumerate is that it returns a DOMRequest. In all other places, DOMRequest refers to a single request; the onsuccess callback is called only once.
For enumerate, the story is different. Enumerate returns somethings where the onsuccess is called for each element. So while you could be tempted to use onsuccess as you used to with other APIs to do async work, this is a complete mistake to do that for the return value of enumerate.
Enumerate should rather return something like {oneach, onerror}. Also, if it did return an array of DOMRequests (or a proxy to around an array of DOMRequests) as I suggested, there would be no problem whatsoever; each file handling would be separated from the enumeration.
Indeed, the main problem with enumerate is that it forces onsuccess callbacks to deal with both file handling (do something wit cursor.result) and enumeration (choose to .continue or not... which we can expect people to forget and have bugs with).
> I'm sympathic to having a dedicated Storage API and not just a File API :-)
> void addEventListener(DOMString type, EventListener listener, optional
> boolean useCapture);
> void removeEventListener(DOMString type, EventListener listener);
> boolean dispatchEvent(DeviceStorageChangeEvent event);
> => This part can be removed by making DeviceStorage inherit from EventTarget
> I see the idl file does it.
This is just a documentation quirk.
> I don't understand the get/getEditable divide. getEditable is enough, no?
If all you want to do is to read from a file then get is significantly
simpler. getEditable requires two nested asynchronous function calls
before you even have a reference to a Blob object which you can read
from.
> About the DeviceStorageChangeEvent, what does path refer to? The notion of
> "absolute path" is hard to understand given that the Storage API is not a
> File API. If it was the absolute path of the underlying implementation, I
> think it'd be a security leak.
Actually, DeviceStorage is effectively a filesystem API. With some
additional top-level API to get a reference to a filesystem rooted in
your pictures folder.
> On Wed, Nov 7, 2012 at 3:33 AM, David Bruant <bruan...@gmail.com > <mailto:bruan...@gmail.com>> wrote:
> Le 06/11/2012 18:36, David Bruant a écrit :
> Both the enumerate and enumerateEditable could return ES6
> generators [1] instead of a custom thing that mimics the exact
> same behavior. SpiderMonkey already has generators. If WedIDL
> has nothing to use ES generators, let's send feedback to get it.
> For that part, it's been brought to my attention [1] that
> DeviceStorage is asynchronous, so an ES6 iterator (it's the
> interface. generators are an implementation of this interface and
> other things) isn't the right fit.
> I guess I don't understand the point of the "continue".
> Apparently, there is an equivalent construct in IDB. I've seen
> code samples and the weird part is the .continue. Basically, the
> developer has to opt-in to continue enumerate results.
> I think this is because you might need to do other asynchronous things > before getting the next result. Like:
> storage = navigator.getDeviceStorage("images");
> cursor = storage.enumerate(".");
> cursor.onsuccess = function () {
> var file = cursor.result;
> req = new XMLHttpRequest();
> req.open("POST", "/some-service");
> req.onreadystatechange = function () {
> if (req.readyState == 4) {
> cursor.continue();
> }
> };
> var reader = new FileReader();
> reader.onload = function () {
> req.send(reader.result);
> };
> reader.readAsArrayBuffer(file);
> };
> Once you start adding error handling and other stuff the logic could > get pretty complicated, and .continue() makes it more doable.
I see. The intermingling of other async operations and how the enumeration should proceed looks like a footgun to me.
Unlike other APIs, enumerate shouldn't return a DOMRequest. In all other places, DOMRequest refers to a single request; the onsuccess callback is called only once. For enumerate, the story is different. Enumerate returns somethings where the onsuccess is called for each element. So while you could be tempted to use onsuccess as you used to with other APIs to do async work, this is a mistake to do that for the return value of enumerate.
Enumerate should rather return something like {oneach, onerror}. oneach would make very clear that your callback is meant to be called for each file and that someone else (the system in that case) is taking care of the enumeration part so you don't have to worry about it. Very much like Array.prototype.forEach. What you care about is doing something with the different files. You don't care about how the enumeration occurs and you should only care if you plan to abort it (which is not the 80% case in my opinion)
> On Tue, Nov 6, 2012 at 9:36 AM, David Bruant <bruan...@gmail.com> wrote:
>> I don't understand the get/getEditable divide. getEditable is enough, no?
> If all you want to do is to read from a file then get is significantly
> simpler. getEditable requires two nested asynchronous function calls
> before you even have a reference to a Blob object which you can read
> from.
Why would it be the case? Is there any benefit in this indirection for getEditable?
>> About the DeviceStorageChangeEvent, what does path refer to? The notion of
>> "absolute path" is hard to understand given that the Storage API is not a
>> File API. If it was the absolute path of the underlying implementation, I
>> think it'd be a security leak.
> Actually, DeviceStorage is effectively a filesystem API. With some
> additional top-level API to get a reference to a filesystem rooted in
> your pictures folder.
When I look at the entire API, only the DeviceStorageChangeEvent interface manipulates an absolute path; all other functions manipulate "name" and "directory". How does one use DeviceStorageChangeEvent's path to relate to the things manipulated by .get/getEditable/delete/enumerate... ?
> Another small piece of feedback:
> add and addNames just ought to be the same method with an optional > argument.
I guess I have another one. The wikimo page says that addNamed will fail if a file of this name already exists, but there is no way to check if a name exists. So it means that currently, if one wants to check if a name is in the storage already, this person has to pull out the entire file through a "get" call to check.
It might be worth considering adding a .has method
