You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#1287 removes direct access to the value property of Option objects. That caused Ramda to consider all Options to be equal, so the PR also added an equals() method to Option. However, I'm realizing that there's a similar issue with Should.js. Now that it doesn't have access to value, eql() (AKA deepEqual()) no longer throws for two different Options. For example:
// This passes.Option.of(1).should.eql(Option.of(2));
We have tests with assertions like that. I noticed this issue while looking at #1291, where you can see this test:
If I change Option.of('12|45') to something else in the test, the test continues to pass.
It looks like Should.js still distinguishes between empty and non-empty Options, since they have different prototypes.
Instead of using a private property like #value in Option, how about using something like _value or Symbol('value')? I think those would signal effectively that the property isn't meant to be used outside the class.
The text was updated successfully, but these errors were encountered:
#1287 removes direct access to the
value
property ofOption
objects. That caused Ramda to consider allOption
s to be equal, so the PR also added anequals()
method toOption
. However, I'm realizing that there's a similar issue with Should.js. Now that it doesn't have access tovalue
,eql()
(AKAdeepEqual()
) no longer throws for two differentOption
s. For example:We have tests with assertions like that. I noticed this issue while looking at #1291, where you can see this test:
central-backend/test/unit/http/middleware.js
Lines 56 to 63 in ca1652e
If I change
Option.of('12|45')
to something else in the test, the test continues to pass.It looks like Should.js still distinguishes between empty and non-empty
Option
s, since they have different prototypes.Instead of using a private property like
#value
inOption
, how about using something like_value
orSymbol('value')
? I think those would signal effectively that the property isn't meant to be used outside the class.The text was updated successfully, but these errors were encountered: