cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian LeRoux...@brian.io>
Subject Re: Type checking in Cordova JS plugins
Date Fri, 16 Nov 2012 09:13:25 GMT
1. I like the proposal Andrew. We need type checking for sure and making it
optional for debugging even better. The light touch you have shown seems
good enough for now. I'd rather we did not augment natives...and not so
sure what you mean around coercing.

2. Success/error callbacks should be optional, says I.

3. When type errors happen, log & return, call errBack, or throw TypeError?

I am very unsure about this one. Essentially this args checking is more for
our bridge than it is for user space so browser api symmetry less important
to me than doing right by the plugin author. It would seem this would
indicate an log and error callback. But again, not entirely certain that
will yield the most expected behavior. At least w/ throwing a TypeError we
fail noisily. =/




On Fri, Nov 16, 2012 at 8:23 AM, Jesse MacFadyen <purplecabbage@gmail.com>wrote:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message