Skip to content

Use typeof to test booleaness. - #696

Merged
robertknight merged 1 commit into
preactjs:masterfrom
bmeurer:Typeof_Equals_Boolean
May 21, 2017
Merged

Use typeof to test booleaness.#696
robertknight merged 1 commit into
preactjs:masterfrom
bmeurer:Typeof_Equals_Boolean

Conversation

@bmeurer

@bmeurer bmeurer commented May 16, 2017

Copy link
Copy Markdown
Contributor

Using the typeof check is shorter, and in case of Chrome also faster at
this point as shown in https://esbench.com/bench/591b46f299634800a03481f8
(upstream bug to address this at http://crbug.com/v8/6403). I personally
also find the typeof version more readable, as it makes it clear what
the intent was.

Using the typeof check is shorter, and in case of Chrome also faster at
this point as shown in https://esbench.com/bench/591b46f299634800a03481f8
(upstream bug to address this at http://crbug.com/v8/6403). I personally
also find the typeof version more readable, as it makes it clear what
the intent was.
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling 3f38738 on bmeurer:Typeof_Equals_Boolean into 9001436 on developit:master.

@robertknight robertknight left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I ran the benchmark in Chrome, Safari & Firefox. The difference was largest in Firefox, smallest in Safari and mid-way between those in Chrome Dev.

@robertknight
robertknight merged commit c9e331b into preactjs:master May 21, 2017
@developit

Copy link
Copy Markdown
Member

Any impact on filesize? That was the original reasoning behind the strict equality checks, but maybe the impact is small enough it's worthwhile.

@bmeurer
bmeurer deleted the Typeof_Equals_Boolean branch May 22, 2017 04:03
@developit developit added this to the 8.2.0 milestone Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants