Skip to content

Commit

Permalink
Option: fix should.js .eql() behaviour
Browse files Browse the repository at this point in the history
Making Option.value a private field broke should(Option).eql() behaviour.

See: getodk#1287 / 5ef4c72

Closes getodk#1296
  • Loading branch information
alxndrsn committed Nov 14, 2024
1 parent 0c3ec3d commit 6cd32ee
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 10 deletions.
27 changes: 17 additions & 10 deletions lib/util/option.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,35 @@ class Option {
}

class Some extends Option {
#value;
// Should.js uses `for ... in` to compare objects, so while should.js is in
// use an Option's value property must be "enumerable".
//
// See: https://github.com/shouldjs/equal/blob/master/index.js
// See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties
//
// TODO revert from __please_use_get to #value when should.js is removed.
//#value;

/* istanbul ignore next */
constructor(value) {
super();
this.#value = value;
this.__please_use_get = value;
}

get() { return this.#value; }
get() { return this.__please_use_get; }

map(fn) { return Option.of(fn(this.#value)); }
filter(predicate) { return predicate(this.#value) === true ? this : none; } // eslint-disable-line no-use-before-define
map(fn) { return Option.of(fn(this.__please_use_get)); }
filter(predicate) { return predicate(this.__please_use_get) === true ? this : none; } // eslint-disable-line no-use-before-define

orNull() { return this.#value; }
orElse() { return this.#value; }
orElseGet() { return this.#value; }
orThrow() { return this.#value; }
orNull() { return this.__please_use_get; }
orElse() { return this.__please_use_get; }
orElseGet() { return this.__please_use_get; }
orThrow() { return this.__please_use_get; }

isDefined() { return true; }
isEmpty() { return false; }

ifDefined(consumer) { consumer(this.#value); }
ifDefined(consumer) { consumer(this.__please_use_get); }

// Used by ramda for comparing objects.
equals(that) {
Expand Down
36 changes: 36 additions & 0 deletions test/unit/util/option.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,40 @@ describe('(libs/FP) Option type', () => {
});
});
});

describe('should.js interactions', () => {

// N.B. should.equal() is different from should.eql():
//
// * .eql(): check equality using ===
// * .eql(): check equality using "should-equal" module
//
// See: https://www.npmjs.com/package/should-equal

describe('should.eql()', () => {
[
true,
false,
0,
1,
'',
'non-empty string',
].forEach(val => {
it(`should recognise two Options of '${val}' to be equal`, () => {
Option.of(val).should.eql(Option.of(val));
});
});

[
[ 0, 1 ],
[ 0, false ],
[ 0, '' ],
[ false, '' ],
].forEach((a, b) => {
it(`should not recognise Options of '${a}' and '${b}' as equal`, () => {
Option.of(a).should.not.eql(Option.of(b));
});
});
});
});
});

0 comments on commit 6cd32ee

Please sign in to comment.