bgrins / filereader.js

A lightweight wrapper for the JavaScript FileReader interface
http://bgrins.github.com/filereader.js/
MIT License
411 stars 86 forks source link

Odd Interface with "opts/readAsMap" #10

Open aaronm67 opened 12 years ago

aaronm67 commented 12 years ago

The interface with the readAsMap is a bit awkward -- typically I'd assume that this:

setupInput(input, {
    readAsMap: { ".*", "DataURL"}
}

would make all inputs return a DataURL, or this:

setupInput(input, {
    readAsMap: { }
}

would make all inputs return the "readAsDefault". Instead, both of these situations end up with a "readAsMap" containing

    {
         'image/*': 'DataURL',
        'text/*' : 'Text'
    }
bgrins commented 12 years ago

I agree - this is confusing. The other issue here is that the order of enumeration is not guaranteed: https://github.com/bgrins/filereader.js/blob/master/filereader.js#L255.

So if you had something like:

readAsMap: {
  "image/png", "DataURL",
  "image/*", "BinaryString",
}

The image/* could potentially be matched first depending on the order the readAsMap is enumerated.

What do you think the ideal behavior / interface for this is?

aaronm67 commented 12 years ago

jQuery "Converters" on Ajax requests are fairly similar -- The interface they provide is similar to this:

   var readAsDefaultMap = { ... };
    if (typeof(opts.converters) == 'undefined') {
        opts.readAsMap = readAsDefaultMap;
    }

The ordering problem could be solved by disallowing Regex in these maps, i.e. "image/*" is disallowed, then rather than looping through the map, it would be accessed directly by key (i.e. readAsMap[type] || readAsDefault).

Alternatively, you could do something to this effect, where you will match the exact content type before trying to match with a regex:

for (var r in readAsMap) {
    if (type ===r) {
        return 'readAs' + readAsMap[r];
    }
}
for (var r in readAsMap) {
    if (type.match(new RegExp(r))) {
        return 'readAs' + readAsMap[r];
    }
}

This doesn't really fix the issue, though, it just makes it a little less common (it will still, for example, pop up for { "image/" : "DataUrl", "image/jp", "BinaryString" }).