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

Improve typing for ==, != expressions #5840

Merged
merged 3 commits into from
Dec 12, 2017
Merged

Conversation

anandthakker
Copy link
Contributor

@anandthakker anandthakker commented Dec 11, 2017

Closes #5761
Closes #5835

The overloads of == and != are now effectively:

  • (T1: Comparable, T2: Comparable) => boolean { T1 == T2 }
  • (Comparable, value) => boolean
  • (value, Comparable) => boolean

Where Comparable = string | number | boolean | null, and evaluation semantics for the value cases are equivalent to Javascript's strict equality (===/!==), i.e., when the value argument's type doesn't match that of the Comparable argument, == evaluates to false, != to true.

An alternative, stricter approach we discussed was to keep the signatures above, but to produce an runtime error in the case of a type mismatch between the value and Comparable arguments -- essentially inserting a type assertion wrapper around the value argument. We went with the less strict approach because:

Closes #5761
Closes #5835

The overloads of == and != are now effectively:
 - `(T1: Comparable, T2: Comparable) => boolean { T1 == T2 }`
 - `(Comparable, value) => boolean`
 - `(value, Comparable) => boolean`

Where `Comparable = string | number | boolean | null`.
Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

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

Per chat, add the alternatives considered and rationale for choosing this one for future us to refer to.

}

possibleOutputs() {
return this.lhs.possibleOutputs().concat(this.rhs.possibleOutputs());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be return [true, false].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️

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.

2 participants