Stage: 1
Champion: Pablo Gorostiaga Belio (@gorosgobe)
Pablo Gorostiaga Belio (@gorosgobe)
Presentations
JavaScript's in
and instanceof
operators have broadly the following behaviour:
a in obj; // returns true if property a is in obj or in its prototype chain, false otherwise
a instanceof C; // returns true if C.prototype is in a's prototype chain, false otherwise
To negate the result of these expressions, we can wrap them with the logical NOT (!
) operator:
!(a in obj);
!(a instanceof C);
Negating an in
/instanceof
expression in this way suffers from a few problems:
Error-proneness1
The logical not operator, !
, has to be applied to the whole expression to produce the intended result. Incorrect parenthesising of the sub-expression (which can be a part of an arbitrarily long expression) and/or applying the !
operator on the wrong operand can lead to errors that are hard to debug, i.e.:
For in
:
if (!a in obj) {
// will not execute, unless obj has a 'true' or 'false' key
// `in` accepts strings or symbols as the LHS parameter, and otherwise coerces all other values to a string
// see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#string_coercion
}
// correct usage
if (!(a in obj)) {
// ...
}
For instanceof
:
if (!a instanceof C) {
// will not execute, unless C provides a @@hasInstance method that returns true for booleans
}
// correct usage
if (!(a instanceof C)) {
// ...
}
This type of error is fairly common. For in
, this Sourcegraph query reveals that there are many instances of this issue (over ~2.1k instances when I ran it) across repos with thousands of stars on GitHub. While there are some false positives (from comments, for example), I highlight some notable examples below:
Examples
Repo | Bugs | Stars | Link | Issue |
---|---|---|---|---|
meteor/meteor | !key in validDevices |
43.6k | Link | Issue |
oven-sh/bun | !"TZ" in process.env |
42.7k | Link | Issue |
SergioBenitez/Rocket | !"message" in msg || !"room" in msg || !"username" in MSG |
21.1k | Link | Issue |
jeromeetienne/AR.js | !'VRFrameData' in window |
15.7k | Link | Issue |
duplicati/duplicati | !'IsUnencryptedOrPassphraseStored' in this.Backup |
9.1k | Link | Issue |
WebKit/WebKit | !'openDatabase' in window |
6.4k | Link | Issue |
buildbot/buildbot | !option in options |
5.1k | Link | Issue |
cloudflare/workerd | !type in this.#recipes |
4.9k | Link | Issue |
muicss/mui | !'rows' in rest |
4.5k | Link | Issue |
jlord/git-it-electron | !'previous' in curCommit |
4.4k | Link | Issue |
zlt2000/microservices-platform | !'onhashchange' in W |
4.2k | Link | Issue |
thechangelog/changelog.com | !"execCommand" in document |
2.6k | Link | Issue |
kiwibrowser/src | !intervalName in this.intervals |
2.3k | Link | Issue |
drawcall/Proton | !'defineProperty' in Object |
2.3k | Link | Issue |
montagejs/collections | !index in this |
2.1k | Link | Issue |
Similarly, for instanceof
, this Sourcegraph query shows that there are also many instances of this bug (~19k occurrences when I ran it). As before, repos with thousands of stars are affected. Some examples follow below:
Examples
Repo | Bugs | Stars | Link | Issue |
---|---|---|---|---|
odoo/odoo | !e instanceof o |
30.1k | Link | Issue |
facebook/flow | !flow instanceof RegExp |
22k | Link | Issue |
v8/v8 | !e instanceof RangeError |
21.5k | Link | Issue |
linlinjava/litemall | !re instanceof RegExp |
18.2k | Link | Issue |
iissnan/hexo-theme-next | !elem instanceof Element |
15.8k | Link | Issue |
chromium/chromium | !this instanceof Test |
15.3k | Link | Issue |
arangodb/arangodb | !context instanceof WebGLRenderingContext |
13.1k | Link | Issue |
ptmt/react-native-macos | !response instanceof Map |
11.3k | Link | N/A (deprecated) |
chakra-core/ChakraCore | !e instanceof TypeError |
8.9k | Link | Issue |
icindy/wxParse | !ext.regex instanceof RegExp |
7.7k | Link | Issue |
WebKit/WebKit | !e instanceof Error |
6.4k | Link | Issue |
golden-layout/golden-layout | !column instanceof lm.items.RowOrColumn |
6k | Link | Issue |
janhuenermann/neurojs | !config instanceof network.Configuration |
4.4k | Link | Issue |
gkz/LiveScript | !last instanceof While |
2.3k | Link | Issue |
CloudBoost/cloudboost | !obj instanceof CB.CloudObject || !obj instanceof CB.CloudFile || !obj instanceof CB.CloudGeoPoint || !obj instanceof CB.CloudTable || !obj instanceof CB.Column |
1.4k | Link | Issue |
Within Bloomberg, we encourage the use of eslint and TypeScript, each of which have an error for these cases. However, because we allow teams to make some of their own decisions about tooling, bugs creeped through: in one large set of internal projects, we found that roughly an eighth of in
/instanceof
usages were negated in
and instanceof
expressions. More than 1% of negated in
uses had this bug. This also affected negated instanceof
, where more than 6% of uses had the bug. Our internal results are aligned with the data from the external sourcegraph queries: there is clearly a higher incidence of the bug on negated instanceof
expressions compared to negated in
expressions. While we are now fixing this internally, overall these results illustrate that this is a common problem due to the lack of ergonomics around negated in
and instanceof
expressions.
The negation of these expressions is not aligned with operators which have a negated version, such as ===
/!==
. This generates confusion among developers and leads to highly upvoted and viewed questions such as Is there a “not in” operator in JavaScript for checking object properties? and Javascript !instanceof If Statement.
To negate the result of an in
/instanceof
expression, we introduce an additional grouping operator (denoted by two parentheses). In addition, the not
is at the beginning of the expression, unlike how this would be read in natural English. Together, both of these factors result in less readable code.
It is common to use in
/instanceof
as a guard in conditionals. Inverting these conditionals to reduce indentation in code, as this is correlated with code complexity, can lead to improved code readability and quality. With the existing operators, inverting the expression in the conditional requires the expression to be both wrapped with parentheses and negated.
!in
, a negated version of in
, where
a !in obj;
is equivalent to
!(a in obj);
!instanceof
, a negated version of instanceof
, where
a !instanceof obj;
is equivalent to
!(a instanceof obj);
- Safer: No longer need to introduce additional grouping, and the negation is applied directly to the operator, as opposed to applying it next to the LHS operand in the expression.
- Improved readability: No longer requires extra grouping to negate the result of the expression. This is aligned with other operators such as
!==
. Reads more naturally and is more intuitive. - Better developer experience: Again, easier to change when refactoring code - a single
!
needs to be added to negate the expression.
Python:
if item not in items:
pass
if ref1 is not ref2:
pass
Kotlin:
if (a !in arr) {}
if (a !is SomeClass) {}
C#:
if (a is not null) {}
Elixir:
a not in [1, 2, 3]
The pattern matching proposal proposes a new relational expression like a in b
or a instanceof b
, using a new operator is
: https://github.com/tc39/proposal-pattern-matching#is-expression
In the same line as in
and instanceof
, we could extend the proposal to include a negated is
operator such as !is
.
Footnotes
-
A note about TypeScript: error-proneness is less of a concern if TypeScript is used, because TypeScript checks that the correct types are passed to the
in
andinstanceof
operators. However, incorrect or lack of types can still cause this issue. This can also happen if you don't use TypeScript, or if that particular part of your code is untyped or usesany
explicitly. ↩