Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor “Loose equality using ==” #4133

Conversation

sideshowbarker
Copy link
Collaborator

@sideshowbarker sideshowbarker commented Apr 15, 2021

This change refactors the entire “Loose equality using ==” section in the “Equality comparisons and sameness” article — with the goal of improving its usability, and at the same time fixing some technical problems with it.

  • Split the giant loose-equality table into a series of individual tables

  • Use &nsbp; in comparison formulas, to prevent bad/ugly breaks that otherwise occur in the tables.

  • Remove all the style attributes from the loose-equality tables (they are unnecessary, and we’d not be able to have styles in it when we move the table source to being markdown).

  • Make it explicit that the comparisons are to the number, bigint, and string primitives — not their corresponding objects. (Otherwise, especially because it has Number, BigInt, String, and Boolean in titlecase, it looked like it was about the objects, not the primitives).

  • Remove unnecessary redundancy/over-emphasis about comparisons to undefined and null: It’s simple:
    undefined == null is true, but undefined or null loosely compared to anything else is false – other than the one exception of document.all, which we deal with in prose.

  • Update the accompanying example to include bigint, and to remove unnecessary and distracting null and undefined comparisons (obviously new String('0') == null and new String('0') == undefined are false; we don’t need to show an example of that).

This change refactors the entire “Loose equality using ==” section in
the “Equality comparisons and sameness” article — with the goal of
improving its usability, and at the same time fixing some technical
problems with it.

* Split the giant loose-equality table into a series of individual tables

* Use &nsbp; in comparison formulas, to prevent bad/ugly breaks that
  otherwise occur in the tables.

* Remove all the style attributes from the loose-equality tables (they
  are unnecessary, and we’d not be able to have styles in it when we
  move the table source to being markdown).

* Make it explicit that the comparisons are to the number, bigint, and
  string *primitives* — not their corresponding objects. (Otherwise,
  especially because it has Number, BigInt, String, and Boolean in
  titlecase, it looked like it was about the objects, not the primitives).

* Remove unnecessary redundancy/over-emphasis about comparisons to
  undefined and null: It’s simple: undefined == null is true, but
  undefined or null loosely compared to anything else is false – other
  than the one exception of document.all, which we deal with in prose.

* Update the accompanying example to include bigint, and to remove
  unnecessary and distracting null and undefined comparisons (obviously
  new String('0') == null and new String('0') == undefined are false; we
  don’t need to show an example of that).
@sideshowbarker sideshowbarker requested a review from a team as a code owner April 15, 2021 14:16
@sideshowbarker sideshowbarker requested review from Rumyra and removed request for a team April 15, 2021 14:16
@wbamberg
Copy link
Collaborator

Thanks for this! I ran out of time today but will take a look tomorrow.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thanks @sideshowbarker ! I must admit I kind of ran out of steam with this page over in #4118, so I'm really happy to see your PR, which is a vast improvement.

I had a few comments/questions.

<table class="standard-table">
<thead>
<tr>
<td>number</td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these be <th>?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should these be <th>?

Indeed yes. Changed them and pushed

</thead>
<tbody>
<tr>
<td><code>ℝ(A)&nbsp;=&nbsp;ℝ(B)</code></td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure why this one (and some other instances further down) seem to be using assignment. AIUI these should all be === or am I confused?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  <td><code>ℝ(A)&nbsp;=&nbsp;ℝ(B)</code></td>

I'm not sure why this one (and some other instances further down) seem to be using assignment. AIUI these should all be === or am I confused?

That’s actually verbatim from step 13 of the “Abstract Equality Comparison” algorithm in the ES spec at https://tc39.es/ecma262/#sec-abstract-equality-comparison:

  1. If Type(x) is BigInt and Type(y) is Number, or if Type(x) is Number and Type(y) is BigInt, then:
  • a. If x or y are any of NaN, +∞𝔽, or -∞𝔽, return false.
  • b. If ℝ(x) = ℝ(y), return true; otherwise return false.

So I think it’s kind of inherently confusing outside of reading it in context the algorithm in the spec — where it’s more clear that it’s an equality comparison, not an assignment operation. But I think it might be wrong to change is to === in MDN, because the operation being performed there isn’t a JavaScript operation on JavaScript operands but instead an spec-internally-defined operation on spec constructs (“mathematical values” https://tc39.es/ecma262/#mathematical-value).

Anyway, I’ve now pushed a change to replace it with ℝ(x) equals ℝ(y). That looks a bit odd but it seems better than keeping ℝ(x) = ℝ(y) as-is and continuing to confuse everybody — and also better than ℝ(x) === ℝ(y), which I’m not sure would be correct (for the reason explained above).

Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW: Perhaps the triple-bar for identity would have been another option: https://en.wikipedia.org/wiki/Equals_sign#Related_symbols

To me as Mathematician, ℝ reads as the „domain of real numbers” (which includes positive and negative infinity, which are excluded in the definition used by TC39 hier).

But I'm not sure, whether I understand the spec hier correctly.

@sideshowbarker sideshowbarker force-pushed the sideshowbarker/equality_comparisons_and_sameness-loose-equality-fixes branch from 980e5b4 to 7583784 Compare April 17, 2021 01:32
@sideshowbarker
Copy link
Collaborator Author

Will, thanks much for the detailed review — I think it’s resulted in some significant improvements. I believe I’ve responded to all the review comments, so this is back to you now for re-review.

@sideshowbarker sideshowbarker requested a review from wbamberg April 17, 2021 02:07
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thanks for the clarifications @sideshowbarker and for dismembering this monster table!

@wbamberg wbamberg merged commit a441e6e into main Apr 17, 2021
@wbamberg wbamberg deleted the sideshowbarker/equality_comparisons_and_sameness-loose-equality-fixes branch April 17, 2021 03:56
@wbamberg
Copy link
Collaborator

With this I think there are now no style attributes left in the JS docs.

@sideshowbarker
Copy link
Collaborator Author

With this I think there are now no style attributes left in the JS docs.

w00t — nice work 🎉

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants