Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Odd Interface with "opts/readAsMap" #10

Open
aaronm67 opened this Issue Jul 27, 2012 · 2 comments

Comments

Projects
None yet
2 participants
Contributor

aaronm67 commented Jul 27, 2012

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'
    }
Owner

bgrins commented Jul 29, 2012

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?

Contributor

aaronm67 commented Jul 31, 2012

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" }).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment