cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesse MacFadyen <purplecabb...@gmail.com>
Subject Re: Type checking in Cordova JS plugins
Date Thu, 15 Nov 2012 21:23:00 GMT
My answers/opinion

1. Up to the plugin developer
2. Up to the plugin developer
3. Up to the plugin developer

However, that does not mean that we can't make the existing APIs
handle params a little more consistently.

Also, we could expose typeChecking helper methods, although they are
pretty easy to come by.
Array.isArray() could be polyfil'd or could be cordova.isArray() ...

Cheers,
  Jesse

Sent from my iPhone5

On 2012-11-15, at 12:16 PM, Andrew Grieve <agrieve@google.com> wrote:

> There's very little consistency when it comes to checking params in plugin
> code.
>
> globalization.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/globalization.js;h=a57062ce8b1e0cc95dde54f8ca600f6ee4876bfd;hb=HEAD>:
> checks every args. logs errors and returns without calling errback, does
> not allow missing errCB.
>
> resolveLocalFileSystemURI.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/resolveLocalFileSystemURI.js;h=c83b0aac3704c46782c3e19876afb180c32fc081;hb=HEAD>:
> Allows missing successCB and errCB. Sanity checks URL has a colon in it.
>
> notification.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/notification.js;h=fbf9046a0e03bb6647219d7201b23f26f4d47084;hb=HEAD>:
> doesn't check types at all
>
> contacts.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/contacts.js;h=2f5a51a20c8ad14b96c4ef935a28686a9bd87eca;hb=HEAD>:
> Throws TypeError if missing successCB, allows missing errCB
>
> compass.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/compass.js;h=c97c1588bf69d2c94cde64736345a7d8b1dc32fe;hb=HEAD>:
> Logs and returns if callbacks are of wrong type. success required, error
> optional.
>
>
> So, the things to agree upon are:
>
> 1. Should we check types or not?
> 2. Success / error callbacks : optional or not?
> 3. When type errors happen, log & return, call errBack, or throw TypeError?
>
>
>
> For some extra inspiration, I played around with what it would look like to
> at least factor out type checking code. Have a look at the
> module<https://github.com/agrieve/incubator-cordova-js/blob/argscheck/lib/common/argscheck.js>and
> it's
> tests<https://github.com/agrieve/incubator-cordova-js/blob/argscheck/test/test.argscheck.js>.
> Also, here's<https://github.com/agrieve/incubator-cordova-js/blob/argscheck/lib/common/plugin/globalization.js>what
> it looks like being used in globalization.js.
>
>
> Now, here's my thoughts for the three questions
> 1. Should we check types or not?
>  - I think it is useful since incorrect args end up in exec() calls and
> are therefore harder to debug when they are wrong.
>  - Perhaps instead of checking types, we add a helper for coercing types?
> This may only apply to strings / numbers though...
>  - I also kind of like the idea of being able to turn off type checking
> for release mode. Having a common function to do the type checking would
> allow us to turn checking on/off for performance.
>
> 2. Success / error callbacks : optional or not?
>  - I'm not positive we need to make these all consistent
>  - If anything, I'd say we'd want them to be optional.
>
> 3. When type errors happen, log & return, call errBack, or throw TypeError?
>  - TypeError is my preference here. This matches what browser APIs do when
> it doesn't like params.

Mime
View raw message