From eb909e82cff184ef74f6f9a7ff42256931e9299c Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Thu, 7 Apr 2016 13:13:04 +0300 Subject: [PATCH] Fix `Match.Optional` to work as it did previously in Meteor 1.2 `Match.Optional` is still only supposed to "pass" if the value is `null` or the specified type. The new `Match.Maybe` allows `undefined` or `null` in addition to the specified types. `Match.Optional` is on the track toward deprecation, however to not break existing code it was *supposed* to stay working the same as before. (per #3876). There weren't tests in place to make sure that `Match.Optional` kept working the same, and the code didn't actually make it keep working the same. Hopefully extra tests will make this better. `.Maybe` has some additional bugs, but should be addressed separately (see #6271) Fixes #6735 --- packages/check/match.js | 4 +++- packages/check/match_test.js | 30 +++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/check/match.js b/packages/check/match.js index d356440ecf..9244fda8ef 100644 --- a/packages/check/match.js +++ b/packages/check/match.js @@ -115,7 +115,9 @@ var Optional = function (pattern) { this.pattern = pattern; }; -var Maybe = Optional; +var Maybe = function (pattern) { + this.pattern = pattern; +}; var OneOf = function (choices) { if (_.isEmpty(choices)) diff --git a/packages/check/match_test.js b/packages/check/match_test.js index ab34d5a3d1..1790dfd175 100644 --- a/packages/check/match_test.js +++ b/packages/check/match_test.js @@ -73,6 +73,7 @@ Tinytest.add("check - check", function (test) { } })); } + if ( type !== null ) fails(null, Match.Optional(type)); // Optional doesn't allow null, but does match on null type fails(pair[0], [type]); fails(pair[0], Object); }); @@ -98,15 +99,42 @@ Tinytest.add("check - check", function (test) { fails({a: 1, b:2}, Match.ObjectIncluding({b: String})); fails({a: 1, b:2}, Match.ObjectIncluding({c: String})); fails({}, {a: Number}); + + // Match.Optional does not match on a null value, unless the allowed type is itself "null" + fails(null, Match.Optional(String)); + fails(null, Match.Optional(undefined)); + matches(null, Match.Optional(null)); + + // on the other hand, undefined, works fine for all of them + matches(undefined, Match.Optional(String)); + matches(undefined, Match.Optional(undefined)); + matches(undefined, Match.Optional(null)); + + fails(true, Match.Optional(String)); // different should still fail + matches("String", Match.Optional(String)); // same should pass + matches({}, {a: Match.Optional(Number)}); matches({a: 1}, {a: Match.Optional(Number)}); fails({a: true}, {a: Match.Optional(Number)}); + fails({a: undefined}, {a: Match.Optional(Number)}); + + // .Maybe requires undefined, null or the allowed type in order to match + matches(null, Match.Maybe(String)); + matches(null, Match.Maybe(undefined)); + matches(null, Match.Maybe(null)); + + matches(undefined, Match.Maybe(String)); + matches(undefined, Match.Maybe(undefined)); + matches(undefined, Match.Maybe(null)); + + fails(true, Match.Maybe(String)); // different should still fail + matches("String", Match.Maybe(String)); // same should pass + matches({}, {a: Match.Maybe(Number)}); matches({a: 1}, {a: Match.Maybe(Number)}); fails({a: true}, {a: Match.Maybe(Number)}); // Match.Optional means "or undefined" at the top level but "or absent" in // objects. - fails({a: undefined}, {a: Match.Optional(Number)}); // Match.Maybe should behave the same as Match.Optional in objects // including handling nulls fails({a: undefined}, {a: Match.Maybe(Number)});