Changing the style guide's preference for loose over strict equality checks in non-test code (was: Re: PSA: The mochitest ise() function is dead, please use is() instead)
On 14/05/2015 01:21, Martin Thomson wrote: On Wed, May 13, 2015 at 4:54 PM, Matthew N. mattn+firefox-...@mozilla.com wrote: In JavaScript, == is preferred to ===. from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators Ahh, that's where it was hiding. Can we reverse that statement please? Concurring with Mike in the other branch of the original thread, I would prefer not to (for non-test code). You've also provided no arguments as to why we should be doing this. ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 14 May 2015, at 03:15, Boris Zbarsky bzbar...@mit.edu wrote: On 5/13/15 7:35 PM, Gregory Szorc wrote: I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual This should be used very very carefully. As a very simple example, using this (or worse yet notDeepEqual) in any test that tries to check for equality of Window objects is not a good idea. The idea is *I think* that ObjectUtils.jsm (which Assert.jsm uses since recently) can be extended to be aware of various object types. Right now it doesn’t have optimal support for iterables like `Map` and `Set`. Same goes for Gecko wrapper objects, like `window`. When you want to use `deepEqual` with those, anyone should feel free to add support for it! Even if it’s only rudimentary support only. Regarding the `==` vs. `===` discussion: I think using `===` for in-test assertions is obviously preferred. This is where Assert.jsm can help and I’d be happy with a patch that replaces `Assert.equal` with `Assert.strictEqual` and just drop the `Assert.strict*` family of methods. In general I think it’s up to a dev’s own discretion which to use; it’s never been a footgun for me because I know each of their semantics inside-out[1]. TBH, I’ve never seen any of my colleagues encounter type coercion bugs over the past eight years. Maybe I’m just lucky, but I do know that the danger of `==` was hyped a bit when it first hit the blogosphere, followed by the (in)famous wtfjs.com site, a couple of years ago and has been a recurring theme ever since. I hope this helps, Mike. [1] No, I’m not trying to pat myself on the back, sometimes this is just the case when you’re a professional ;-) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 14/05/15 00:35, Gregory Szorc wrote: I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual, which uses ObjectUtils.jsm goodness for type aware comparisons so things like Date, RegExp, and Object comparisons have sane behavior. (deepEqual falls back to === for non-special types. If you are writing tests for web-exposed APIs you should strongly consider writing web-platform-tests instead of mochitests. These provide an assert_equals function for equality testing which does what the ES spec calls SameValue equality; like === for most cases but with -0 != 0 and NaN === NaN. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to remove: mochitest-flavor mach commands
On 13/05/15 06:29 PM, Gregory Szorc wrote: On Wed, May 13, 2015 at 2:03 PM, Ms2ger ms2...@gmail.com wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/13/2015 09:54 PM, Andrew Halberstadt wrote: As mentioned previously in another post, work is under way to remove the flavor specific mochitest commands (e.g mach mochitest-plain, mach mochitest-browser etc.). Does this include `mochitest-1` and friends? These are a lot easier to use than having to figure out the exact incantation for the chunked suites. IIRC `mochitest-1` and other job-name based arguments are only exposed/supported via `mach test`. I don't believe Andrew's work will impact these. Yes, those will not be impacted. Those may even be exposed to the |mach mochitest| command in the future. Unrelated side note: I think it would be nice if the harness specific mach commands did all of the leg work and |mach test| just delegated to them. I'm looking into it but haven't yet figured out a good way of doing it. Until then, there will be a slightly different feature set between |mach test| and |mach mochitest|. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 5/14/15 8:03 AM, Mike de Boer wrote: Same goes for Gecko wrapper objects, like `window`. When you want to use `deepEqual` with those, anyone should feel free to add support for it! Here's the thing. When comparing window objects, in what sense would anything other than === be useful? And in general, deepEquals (structural type equality) and === (reference identity equality) are just completely different operations for objects. Sometimes you want one, sometimes the other. In practice, for tests of the DOM, structural type equality is almost never the desired type of equality. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 5/14/15 4:22 PM, Gijs Kruitbosch wrote:
On 14/05/2015 19:08, Martin Thomson wrote:
If you intend to support false and null and undefined and NaN and
attribute the same semantics, _say so_. Make it explicit. Don't make
me (the person reading your code years later) that this is your
intent.
Then why is if (!foo) OK and do you not prefer:
if (foo === false || foo === undefined || foo === || ...)
? Why do I not have to make it explicit (which is a word I think
doesn't make sense here, see below) in that case?
Exactly. Nobody wants to outlaw if (foo) and if (!foo). Why is that?
Because JS comes with too many ways to say no: undefined, false, null,
Nan, '', 0 (and they will all happen!)
More often than not, treating them all the same semantically (falsy) is
more likely to be correct (and implicitly safer by being less verbose
and you wont remember them all otherwise):
foo(bar) {
if (!bar) {
baz();
} else {
fum(bar);
}
}
foo({});
foo(null);
foo();
If we accept that statement, that testing for falsy is safer than
testing the five no's explicitly, then the safer comparators seem to be:
== and !=.
e.g. isn't
foo(bar) {
if (bar == null) {
baz();
} else {
fum(bar);
}
}
foo({});
foo(null);
foo();
safer than:
foo(bar) {
if (bar === null) {
baz();
} else {
fum(bar);
}
}
foo({});
foo(null);
foo(); // ugh undefined is more like null than {}!
?
Sure, we shouldn't write either, but then what about:
foo(x,y) {
if (x != y) {
baz();
} else {
fum(x);
}
}
foo({}, {});
foo({}, null);
foo({});
foo(null, null);
foo(undefined, null);
foo(null);
?
.: Jan-Ivar :.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 5/14/15 1:35 PM, Gijs Kruitbosch wrote:
var foo = [1,2,3];
window.open('bar.html', '_blank', '', foo);
in bar.html, checking the type of foo using instanceof with Array fails.
For builtin Arrays we now have isArray, but this does not work for
custom JS classes or DOM elements.
Actually, DOM constructors have an instanceof behavior that is
global-agnostic. So instanceof actually does something sane if the RHS
is a built-in DOM constructor.
For JS classes you still have problems, of course.
-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 14/05/2015 18:12, Martin Thomson wrote:
On Thu, May 14, 2015 at 9:40 AM, Gijs Kruitbosch
gijskruitbo...@gmail.com wrote:
Crockford offers plenty of
reasons in his book.
I've not read Crockford's book and have no plans to, but there are plenty of
reasons against, too. :-)
Do you want me to send you a copy?
No, but thank you for the offer.
then that does not mean I don't care or don't know - I *know* that
ary.length is a number, and there's no reason to bother with writing ===.
That just says that you are too lazy to hit the key one more time.
And that you don't care to signal to others that you know this.
I mean, obviously the example is simplified. You seem to think that
=== means I'm sure this will be the right type. In the same way you
imply that == indicates uncertainty about type (or acceptance of
multiple types), I would argue that when I see === I get suspicious
about whether the code could be failing because it gets passed a
similar-but-different-type value (false vs. null vs. undefined vs. NaN
especially).
Likewise, if I know an argument to a function is either not present or a
non-empty string, and I do:
if (arg) {
dosomethingwitharg(arg);
}
I'm actually perfectly fine with this form. Including something in a
conditional statement is pretty damned explicit as far as I'm
concerned. It's at least as clear as if (!!arg) and easier to read.
(I have a personal bugbear against the falsy-ness of the empty string,
but I can concede that point.)
I don't understand the distinction you're making, then; why is the type
coercion OK here but not for == ? Surely this is being just as lazy as
the other case?
I've never seen case
where an explicit check is not possible, and does not help make code
clearer.
I disagree. new String() is my most hated example (foo == new
String(foo)) but (foo !== new String(foo)), but also in places where
we use implicit truthy/falsy checks, the shorthand 'if (foo)' and/or 'if
(!foo)' is much more useful, clear and concise than trying to check for all
the ways in which foo could be falsy/truthy (and inevitably forgetting
something silly like NaN or types from other windows when using instanceof
or ...).
I have no idea what you are talking about. I have never had cause to
use new String() anywhere. .toString() maybe.
There are more than 1000 hits for new String( in MXR, so our codebase
disagrees.
x = foo;
if (x === foo) { ... } // is fine
You're arguing that === always makes code clearer, and that explicit
checks are always possible. I disagree with both.
As noted above, === creates just as many uncertainties as == for the
reader. The solution is testing (and in some cases comments), whichever
operator we pick.
Expanding checks which are simple when using == in order to use === is
not always clearer, likewise for other type coercion (where you seem to
be OK with it, which I find hard to understand).
There are cases where explicit checks are impossible. String objects are
wonky and don't compare well, as noted earlier. Likewise, in the
following case:
var foo = [1,2,3];
window.open('bar.html', '_blank', '', foo);
in bar.html, checking the type of foo using instanceof with Array fails.
For builtin Arrays we now have isArray, but this does not work for
custom JS classes or DOM elements.
Also, if you're very serious about wanting to effect this change, you
should probably take this to fx-dev, as the style guide is specifically
about chrome/firefox JS.
~ Gijs
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On Thu, May 14, 2015 at 10:35 AM, Gijs Kruitbosch
gijskruitbo...@gmail.com wrote:
I mean, obviously the example is simplified. You seem to think that ===
means I'm sure this will be the right type. In the same way you imply that
== indicates uncertainty about type (or acceptance of multiple types), I
would argue that when I see === I get suspicious about whether the code
could be failing because it gets passed a similar-but-different-type value
(false vs. null vs. undefined vs. NaN especially).
If you intend to support false and null and undefined and NaN and
attribute the same semantics, _say so_. Make it explicit. Don't make
me (the person reading your code years later) that this is your
intent.
if (arg) {
dosomethingwitharg(arg);
}
I'm actually perfectly fine with this form. Including something in a
conditional statement is pretty damned explicit as far as I'm
concerned. It's at least as clear as if (!!arg) and easier to read.
(I have a personal bugbear against the falsy-ness of the empty string,
but I can concede that point.)
I don't understand the distinction you're making, then; why is the type
coercion OK here but not for == ? Surely this is being just as lazy as the
other case?
Because it's explicit. if () takes a boolean.
You're arguing that === always makes code clearer, and that explicit checks
are always possible. I disagree with both.
Looks like we're in for some more disagreeing then.
As noted above, === creates just as many uncertainties as == for the reader.
The solution is testing (and in some cases comments), whichever operator we
pick.
No, you are suggesting that === cuts out some code paths or options
that might be wanted. But it doesn't, it only forces you to be
explicit about your intent in including those options.
in bar.html, checking the type of foo using instanceof with Array fails. For
builtin Arrays we now have isArray, but this does not work for custom JS
classes or DOM elements.
That's a legacy failure, but not one to solve with further obfuscation.
Also, if you're very serious about wanting to effect this change, you should
probably take this to fx-dev, as the style guide is specifically about
chrome/firefox JS.
I'm serious.
I originally posted to fx-dev. You appear to have removed it from CC.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On Thu, May 14, 2015 at 9:40 AM, Gijs Kruitbosch
gijskruitbo...@gmail.com wrote:
Do you think the people that wrote the style guide (not me, so I'm not
trying to be defensive over the document, to be clear) did not understand
what they wrote? :-)
No, but I do think that they were wrong.
Crockford offers plenty of
reasons in his book.
I've not read Crockford's book and have no plans to, but there are plenty of
reasons against, too. :-)
Do you want me to send you a copy?
then that does not mean I don't care or don't know - I *know* that
ary.length is a number, and there's no reason to bother with writing ===.
That just says that you are too lazy to hit the key one more time.
And that you don't care to signal to others that you know this.
Likewise, if I know an argument to a function is either not present or a
non-empty string, and I do:
if (arg) {
dosomethingwitharg(arg);
}
I'm actually perfectly fine with this form. Including something in a
conditional statement is pretty damned explicit as far as I'm
concerned. It's at least as clear as if (!!arg) and easier to read.
(I have a personal bugbear against the falsy-ness of the empty string,
but I can concede that point.)
I've never seen case
where an explicit check is not possible, and does not help make code
clearer.
I disagree. new String() is my most hated example (foo == new
String(foo)) but (foo !== new String(foo)), but also in places where
we use implicit truthy/falsy checks, the shorthand 'if (foo)' and/or 'if
(!foo)' is much more useful, clear and concise than trying to check for all
the ways in which foo could be falsy/truthy (and inevitably forgetting
something silly like NaN or types from other windows when using instanceof
or ...).
I have no idea what you are talking about. I have never had cause to
use new String() anywhere. .toString() maybe.
x = foo;
if (x === foo) { ... } // is fine
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: The mochitest ise() function is dead, please use is() instead
On 2015-05-14 8:03 AM, Mike de Boer wrote: On 14 May 2015, at 03:15, Boris Zbarsky bzbar...@mit.edu wrote: On 5/13/15 7:35 PM, Gregory Szorc wrote: I would steer people in the direction of Assert.jsm, specifically Assert.deepEqual This should be used very very carefully. As a very simple example, using this (or worse yet notDeepEqual) in any test that tries to check for equality of Window objects is not a good idea. The idea is *I think* that ObjectUtils.jsm (which Assert.jsm uses since recently) can be extended to be aware of various object types. Right now it doesn’t have optimal support for iterables like `Map` and `Set`. Same goes for Gecko wrapper objects, like `window`. When you want to use `deepEqual` with those, anyone should feel free to add support for it! Even if it’s only rudimentary support only. But the point is that there is no use to deep comparing something like two window objects. If you are doing that, you're almost definitely doing something wrong. This is why usage of these Assert.jsm functions should be highly discouraged at least in tests examining the Web platform. The semantics are too hard to understand, and they may not be what we want (as is the case with the deep equality checks) at all. (FWIW, I noticed recently that someone had edited the Mochitest MDN page claiming that the usage of Assert.jsm is encouraged. I have updated that page to match the reality.) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 14/05/2015 17:14, Martin Thomson wrote:
On Thu, May 14, 2015 at 5:17 AM, Gijs Kruitbosch
gijskruitbo...@gmail.com wrote:
On 14/05/2015 01:21, Martin Thomson wrote:
On Wed, May 13, 2015 at 4:54 PM, Matthew N.
mattn+firefox-...@mozilla.com wrote:
In JavaScript, == is preferred to ===. from
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators
Ahh, that's where it was hiding. Can we reverse that statement please?
Concurring with Mike in the other branch of the original thread, I would
prefer not to (for non-test code). You've also provided no arguments as to
why we should be doing this.
I thought that this was understood.
Do you think the people that wrote the style guide (not me, so I'm not
trying to be defensive over the document, to be clear) did not
understand what they wrote? :-)
Crockford offers plenty of
reasons in his book.
I've not read Crockford's book and have no plans to, but there are
plenty of reasons against, too. :-)
To summarize, == uses implicit type coercion.
I think everyone reading this thread, and certainly everyone who wrote
that style guide, is aware of this.
It says that you don't care or that you don't know what something is.
Well, no, not exactly. If I write:
var ary = a,b,c.split(',')
if (ary.length == 2)
then that does not mean I don't care or don't know - I *know* that
ary.length is a number, and there's no reason to bother with writing ===.
Likewise, if I know an argument to a function is either not present or a
non-empty string, and I do:
if (arg) {
dosomethingwitharg(arg);
}
then that doesn't mean I don't know or care about the implicit type
coercion which happens, just that it would be more arduous to write
comparisons with undefined.
In fact, in general I would say we write APIs and functions and so on
where the type is meant to not matter, and having any where different
values that are loosely but not strictly equivalent have different
meanings is frowned upon. In other words, we should strive for code
where the comparison operator used does not matter.
The only real reason I've seen to pick === over == is performance, and
most of our chrome JS code is not in hot code paths, and we have other
overhead (XPCOM calls, DOM calls, layout flushes) which are a much
bigger perf issue.
At a more meta-level, I would personally also argue that the spirit of
JS-the-language is one where type checks are rare and therefore allow
easily mocking/replacing real things with lookalike things (I think
the patch I wrote here:
https://bugzilla.mozilla.org/show_bug.cgi?id=341767 is still one of my
favourite examples of (ab)using this tendency). Strict checks throw that
kind of thing away, and I think that would be a net loss (when not
writing code where perf is paramount, for which we have things like asmjs).
I've never seen case
where an explicit check is not possible, and does not help make code
clearer.
I disagree. new String() is my most hated example (foo == new
String(foo)) but (foo !== new String(foo)), but also in places
where we use implicit truthy/falsy checks, the shorthand 'if (foo)'
and/or 'if (!foo)' is much more useful, clear and concise than trying to
check for all the ways in which foo could be falsy/truthy (and
inevitably forgetting something silly like NaN or types from other
windows when using instanceof or ...).
~ Gijs
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code (was: Re: PSA: The mochitest ise() function is dead, please use is() instead)
On Thu, May 14, 2015 at 9:14 AM, Martin Thomson m...@mozilla.com wrote: On Thu, May 14, 2015 at 5:17 AM, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: On 14/05/2015 01:21, Martin Thomson wrote: On Wed, May 13, 2015 at 4:54 PM, Matthew N. mattn+firefox-...@mozilla.com wrote: In JavaScript, == is preferred to ===. from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators Ahh, that's where it was hiding. Can we reverse that statement please? Concurring with Mike in the other branch of the original thread, I would prefer not to (for non-test code). You've also provided no arguments as to why we should be doing this. I thought that this was understood. Crockford offers plenty of reasons in his book. To summarize, == uses implicit type coercion. It says that you don't care or that you don't know what something is. I've never seen case where an explicit check is not possible, and does not help make code clearer. FWIW, I concur with Martin for the reasons he states. I don't think anyone is saying that we should change existing code, but this would be clearer for new code. -Ekr ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code (was: Re: PSA: The mochitest ise() function is dead, please use is() instead)
On Thu, May 14, 2015 at 5:17 AM, Gijs Kruitbosch gijskruitbo...@gmail.com wrote: On 14/05/2015 01:21, Martin Thomson wrote: On Wed, May 13, 2015 at 4:54 PM, Matthew N. mattn+firefox-...@mozilla.com wrote: In JavaScript, == is preferred to ===. from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators Ahh, that's where it was hiding. Can we reverse that statement please? Concurring with Mike in the other branch of the original thread, I would prefer not to (for non-test code). You've also provided no arguments as to why we should be doing this. I thought that this was understood. Crockford offers plenty of reasons in his book. To summarize, == uses implicit type coercion. It says that you don't care or that you don't know what something is. I've never seen case where an explicit check is not possible, and does not help make code clearer. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 05/14/2015 04:22 PM, Gijs Kruitbosch wrote: == is not any less explicit than ===. Both versions have an exact, specified meaning. They both have exact meanings. But people, especially new contributors new to JS, frequently use == without understanding the nuances and footguns involved. In Firefox OS/Gaia I think the emergent convention[1] is that you should always use === unless you have a specific need to use == and in that case you should have a comment explaining exactly why you want the coercing behaviour. I, at least, do this in all my reviews. And I find this disambiguates things nicely and helps me as a reviewer in understanding the expected data-flow of the code I'm reviewing. Andrew 1: I could swear we have a style-guide for Gaia but I can't find it. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Question: How to spawn ServiceWorkers in the correct child on e10s and b2g?
Hello, As part of allowing various APIs (Push notifications, Notifications, BackgroundSync etc.) to use ServiceWorkers, we need some way to start them in the child process. I'm trying to figure out how to do this properly in e10s and on b2g. Here is an outline, I'm hoping someone with more knowledge will help fill in the gaps. The way we do this right now is to insert a frame script in all the children that listens to a broadcast event sent by the message manager, but eventually we may have sensitive data in the messages and would prefer to send it only to the correct child. 1. What information do dependent APIs need to store? I can think of: a) appID b) inBrowserElement c) origin d) scope e) (optional) manifestURL when appID is not UNKNOWN? Can any or all of these be replaced by a nsIPrincipal and something else? 2. When the parent process wants to wake up a SW and send it an event, we need to implement the following algorithm: If there is a child process capable of handling the set of data in step 1: send event to that child process. else: create a new child process with the data in step 1 (and presumably the privileges get set accordingly) send this child process the message From my limited reading of ContentParent and friends, this is what I understand: 1) Use ContentParent::GetAll() to get existing content processes. 2) On each ContentParent get the list of TabContexts 3) Ask each TabContext if it can deal with this event using something like AssertAppProcess - https://dxr.mozilla.org/mozilla-central/source/dom/ipc/AppProcessChecker.cpp#76 4) If some TabContext replies in the affirmative, send it a message (How do I do this?) 5) Otherwise, create a new content process - I'm looking at ContentParent::CreateBrowserOrApp() Ideally, I'd like to do all of this from privileged JS, but it should be easy to wrap if it isn't right now. Best, Nikhil ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 14/05/2015 19:08, Martin Thomson wrote:
On Thu, May 14, 2015 at 10:35 AM, Gijs Kruitbosch
gijskruitbo...@gmail.com wrote:
I mean, obviously the example is simplified. You seem to think that ===
means I'm sure this will be the right type. In the same way you imply that
== indicates uncertainty about type (or acceptance of multiple types), I
would argue that when I see === I get suspicious about whether the code
could be failing because it gets passed a similar-but-different-type value
(false vs. null vs. undefined vs. NaN especially).
If you intend to support false and null and undefined and NaN and
attribute the same semantics, _say so_. Make it explicit. Don't make
me (the person reading your code years later) that this is your
intent.
Then why is if (!foo) OK and do you not prefer:
if (foo === false || foo === undefined || foo === || ...)
? Why do I not have to make it explicit (which is a word I think
doesn't make sense here, see below) in that case?
if (arg) {
dosomethingwitharg(arg);
}
I'm actually perfectly fine with this form. Including something in a
conditional statement is pretty damned explicit as far as I'm
concerned. It's at least as clear as if (!!arg) and easier to read.
(I have a personal bugbear against the falsy-ness of the empty string,
but I can concede that point.)
I don't understand the distinction you're making, then; why is the type
coercion OK here but not for == ? Surely this is being just as lazy as the
other case?
Because it's explicit. if () takes a boolean.
See above.
As noted above, === creates just as many uncertainties as == for the reader.
The solution is testing (and in some cases comments), whichever operator we
pick.
No, you are suggesting that === cuts out some code paths or options
that might be wanted. But it doesn't, it only forces you to be
explicit about your intent in including those options.
== is not any less explicit than ===. Both versions have an exact,
specified meaning. Representing the meaning of == as a con/disjunction
of === statements will of necessity be more verbose.
Alternatively, if you only need a subset of the == meaning, you will
restrict the conditions under which the expression returns true, but it
is not being more explicit.
It is saying something different, much like foo bar is not the same
as foo = bar - given the same foo, the first expression will just be
true in a subset of the cases where the second expression is true.
in bar.html, checking the type of foo using instanceof with Array fails. For
builtin Arrays we now have isArray, but this does not work for custom JS
classes or DOM elements.
That's a legacy failure, but not one to solve with further obfuscation.
I will again say that obfuscation and explicit is not what is at
issue here, IMO. The meaning of == is perfectly clear and specified,
just like ===. The cases where the result is unexpected is where
people have not thought through the values on which the operator, err,
operates. Such cases exist for both operators. There might seem to be
more for == if and only if you show people literals, like wtfjs and
friends. My point is there will be just as many unexpected issues when
you use === on opaque variables, because you didn't expect that that
code in that other file set this thing to null instead of deleting the
property (causing it to not be === undefined in your comparison) -- for
example.
Maybe there is a difference with firefox browser JS and platform JS here
(on which you seem to be working), in terms of which kinds of type
confusion are likely and that need thinking about, but IMO that just
strengthens Mike's case that we should let people write whatever they're
comfortable with.
Also, if you're very serious about wanting to effect this change, you should
probably take this to fx-dev, as the style guide is specifically about
chrome/firefox JS.
I'm serious.
I originally posted to fx-dev. You appear to have removed it from CC.
Oh shoot. Sorry. Re-added; I read this list over news:, and somehow the
mailing list responses include both the ng and the identical ml, and I
must have gotten confused and removed both fx-dev and the dev-platform
list (I do need to remove the identical ml, otherwise my replies show up
twice). Apologies.
~ Gijs
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Question: How to spawn ServiceWorkers in the correct child on e10s and b2g?
On Thu, May 14, 2015 at 12:18 PM, nsm.nik...@gmail.com wrote: 1. What information do dependent APIs need to store? I can think of: a) appID b) inBrowserElement c) origin d) scope e) (optional) manifestURL when appID is not UNKNOWN? Can any or all of these be replaced by a nsIPrincipal and something else? Serendipitously, I'm making this situation a lot easier in bug 1163254. The idea is that nsIPrincipal::origin will also serialize all of the security-sensitive differentiators (appId, inBrowserElement, etc), so you can just consult that if you need a canonical stringified representation of the origin. If you eventually need to convert it _back_ to an nsIPrincipal, you should probably use the nsISerializable stuff instead. It's a bit of a pain to use right now, though we could add a helper. bholley ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 5/14/15 16:33, Gijs Kruitbosch wrote: Can you give a concrete example where you had to change a contributor's patch in frontend gaia code to prefer === to prevent real bugs? From what I've seen, it's typically a matter of making the results unsurprising for subsequent code maintainers, because the rules of what gets coerced to what are not intuitive. I'll crib from Crockford's examples (cf. Appendix B: The Bad Parts from JavaScript: The Good Parts). How many of these can you correctly predict the result of? 1. '' == '0' 2. 0 == '' 3. 0 == '0' 4. false == 'false' 5. false == '0' 6. false == undefined 7. false == null 8. null == undefined 9. '\t\r\n' == 0 I've posted the answers at https://pastebin.mozilla.org/8833537 If you had to think for more than a few moments to reach the right conclusion about any of these -- or, heaven forbid, actually got one wrong -- then I think you need to ultimately concede that the use of == is more confusing than it needs to be. -- Adam Roach Principal Platform Engineer a...@mozilla.com +1 650 903 0800 x863 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 14/05/2015 23:15, Adam Roach wrote: On 5/14/15 16:33, Gijs Kruitbosch wrote: Can you give a concrete example where you had to change a contributor's patch in frontend gaia code to prefer === to prevent real bugs? From what I've seen, it's typically a matter of making the results unsurprising for subsequent code maintainers, because the rules of what gets coerced to what are not intuitive. This isn't what I asked, though. I have seen the argument you're giving (and Crockford's and similar examples) many times - they're fun party games - but never cases in the real world where this is an issue. I'll crib from Crockford's examples (cf. Appendix B: The Bad Parts from JavaScript: The Good Parts). How many of these can you correctly predict the result of? 1. '' == '0' 2. 0 == '' 3. 0 == '0' 4. false == 'false' 5. false == '0' 6. false == undefined 7. false == null 8. null == undefined 9. '\t\r\n' == 0 Our code style means none of 4-7 should actually ever happen - it explicitly says, do not compare directly to |true| or |false|. If given the following examples: foo === undefined foopy === bar bar === null blah === 0 how many can you correctly predict? None, because there is no context where the variables came from. Nobody ever compares two literals to each other, and because our codebase has no type annotations, you never have enough information to decide what the result of the comparison is without knowing context, irrespective of which operator you use. If I have a function signature like this: function foo(x, y, z) and these calls: foo(x); foo(x, null, z); foo(x, uriRef, z); (based on actual examples of some of our URI loading wrappers in browser frontend code) inside foo, I really don't want to use strict equality for checking if y is present or not. Checking against null or undefined would fail in at least some of the cases. ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 14/05/2015 21:45, Andrew Sutherland wrote: On 05/14/2015 04:22 PM, Gijs Kruitbosch wrote: == is not any less explicit than ===. Both versions have an exact, specified meaning. They both have exact meanings. But people, especially new contributors new to JS, frequently use == without understanding the nuances and footguns involved. In Firefox OS/Gaia I think the emergent convention[1] is that you should always use === unless you have a specific need to use == and in that case you should have a comment explaining exactly why you want the coercing behaviour. I, at least, do this in all my reviews. And I find this disambiguates things nicely and helps me as a reviewer in understanding the expected data-flow of the code I'm reviewing. Can you give a concrete example where you had to change a contributor's patch in frontend gaia code to prefer === to prevent real bugs ? As I noted in a snipped part of my message, I can believe that different parts of the codebases have different needs here - I've just had the opposite experience where I don't recall running into issues of the type you describe, but rather things where contributors use: if (typeof x == undefined || x == false || x == ) or similar, and I suggest if (!x). While I agree we should do our best to welcome new contributors, I don't know that having our style guide dictate use === instead of == would be helpful. Andrew 1: I could swear we have a style-guide for Gaia but I can't find it. https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Understanding_the_Gaia_codebase#Gaia_coding_style which doesn't seem to mention this, but defers in part to the general Mozilla code style (but not the section where this line is hiding). ~ Gijs ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
How many of these can you correctly predict the result of? Knowing Gijs, I know the answer is all of them :) This is certainly a factor in this discussion - how familiar you are with JS, and how much you use it every day, certainly affects your perspective. Gijs' (and my) experience is that using === instead of == doesn't actually solve a lot of problems in practice. At least not for our project or our team. That being said, I don't think there is a strong argument against using === consistently, and it very well may help avoid trip-ups in some cases, particularly with those not as familiar with JS' quirks. But it's not such a big win that it's worth reformatting all our code (which I appreciate no one was proposing), and it might not even be a big enough with to be worth the attention it's gotten in this thread. Gavin On Thu, May 14, 2015 at 3:15 PM, Adam Roach a...@mozilla.com wrote: On 5/14/15 16:33, Gijs Kruitbosch wrote: Can you give a concrete example where you had to change a contributor's patch in frontend gaia code to prefer === to prevent real bugs? From what I've seen, it's typically a matter of making the results unsurprising for subsequent code maintainers, because the rules of what gets coerced to what are not intuitive. I'll crib from Crockford's examples (cf. Appendix B: The Bad Parts from JavaScript: The Good Parts). How many of these can you correctly predict the result of? 1. '' == '0' 2. 0 == '' 3. 0 == '0' 4. false == 'false' 5. false == '0' 6. false == undefined 7. false == null 8. null == undefined 9. '\t\r\n' == 0 I've posted the answers at https://pastebin.mozilla.org/8833537 If you had to think for more than a few moments to reach the right conclusion about any of these -- or, heaven forbid, actually got one wrong -- then I think you need to ultimately concede that the use of == is more confusing than it needs to be. -- Adam Roach Principal Platform Engineer a...@mozilla.com +1 650 903 0800 x863 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Changing the style guide's preference for loose over strict equality checks in non-test code
On 14/05/2015 22:55, Eric Rescorla wrote: On Thu, May 14, 2015 at 1:22 PM, Gijs Kruitbosch gijskruitbo...@gmail.com mailto:gijskruitbo...@gmail.com wrote: Maybe there is a difference with firefox browser JS and platform JS here (on which you seem to be working), in terms of which kinds of type confusion are likely and that need thinking about, but IMO that just strengthens Mike's case that we should let people write whatever they're comfortable with. I'd like to make sure I understand what you're arguing for. Is it: - We should retain this section of the style guide (recommending for == rather than ===) From my personal POV, I would prefer this. I remain un-===-converted so far, but I bet the opposite can be said for people on the === side of the discussion. It seems unlikely we'll reach consensus. - We should remove this section of the style guide and make no recommendation. It seems there are some/many people who feel strongly about this and who are unhappy with the recommendation of ==. If we want it changed, I would prefer explicitly mentioning that we're leaving it up to the author/reviewer/project/file style. ~ Gijs Thanks, -Ekr ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
