Skip to content

Commit

Permalink
Fix helper tests
Browse files Browse the repository at this point in the history
  • Loading branch information
thinkingserious committed Aug 25, 2017
1 parent 02ff533 commit 74e3270
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions packages/helpers/classes/personalization.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class Personalization {
if (typeof substitutions === 'undefined') {
return;
}
if (typeof substitutions !== 'object') {
if (typeof substitutions !== 'object' || substitutions === null) {
throw new Error('Object expected for `substitutions`');
}
this.substitutions = substitutions;
Expand All @@ -244,10 +244,10 @@ class Personalization {
* Reverse merge substitutions, preserving existing ones
*/
reverseMergeSubstitutions(substitutions) {
if (typeof substitutions === 'undefined' || substitutions === null) {
if (typeof substitutions === 'undefined') {
return;
}
if (typeof substitutions !== 'object') {
if (typeof substitutions !== 'object' || substitutions === null) {
throw new Error('Object expected for `substitutions`');
}
this.substitutions = Object.assign({}, substitutions, this.substitutions);
Expand Down

2 comments on commit 74e3270

@adamreisnz
Copy link
Contributor

@adamreisnz adamreisnz commented on 74e3270 Aug 25, 2017

Choose a reason for hiding this comment

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

@thinkingserious What was the reason for this change? If it's just failing tests, then I think the tests need to be updated. This was something that I recall adding to fix a bug. Null is a valid value for substitutions, if you don't want them done, so it shouldn't throw an error on you.

@thinkingserious
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake, reverted.

Please sign in to comment.