bugfix & setup to replace 3p helpers incrementally

bigcommerce · Jul 8, 2022 · 0a79e62 · 0a79e62
1 parent eff3248
commit 0a79e62
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 31 deletions.
diff --git a/helpers.js b/helpers.js
@@ -11,7 +11,7 @@ const helpersList = [
     'dynamicComponent',
     'encodeHtmlEntities',
     'for',
-    'get',
+    // 'get',
     'getContentImage',
     'getContentImageSrcset',
     'getFontLoaderConfig',
@@ -21,7 +21,7 @@ const helpersList = [
     'getImageManagerImageSrcset',
     'getImageSrcset',
     'getImageSrcset1x2x',
-    'getObject',
+    // 'getObject',
     'getVar',
     'helperMissing',
     'if',

diff --git a/helpers/get.js b/helpers/get.js
@@ -12,11 +12,21 @@ const factory = (globals) => {
     return function (path, context) {
         let options = arguments[arguments.length - 1];
 
+        // for backwards compatibility
+        if (context === null || (typeof context !== 'object' && typeof context !== 'function')) {
+            return context;
+        }
+
         // use an empty context if none was given
-        if (arguments.length < 2) {
+        if (!context) {
             context = {};
         }
 
+        // for backwards compatibility: safely use options hash as context if no context was passed
+        if (arguments.length < 3) {
+            context = { hash: options.hash };
+        }
+
         let value = getValue(globals, context, path);
         if (options && options.fn) {
             return value ? options.fn(value) : options.inverse(context);

diff --git a/helpers/lib/common.js b/helpers/lib/common.js
@@ -18,6 +18,11 @@ function isValidURL(val) {
 function getValue(globals, object, path) {
     let parts;
 
+    // for backwards compatibility
+    if (!path) {
+        return object;
+    }
+
     // unwrap Handlebars.SafeString for compatibility with `concat` etc.
     path = unwrapIfSafeString(globals.handlebars, path);
 
@@ -28,7 +33,7 @@ function getValue(globals, object, path) {
         parts = path;
     } else {
         let key = String(path);
-        return Object.prototype.hasOwnProperty.call(object, key) ? object[key] : undefined;
+        return Object.keys(object).indexOf(key) !== -1 ? object[key] : undefined;
     }
 
     let result = object;
@@ -40,7 +45,7 @@ function getValue(globals, object, path) {
             continue;
         }
         key = prefix + key;
-        if (Object.prototype.hasOwnProperty.call(result, key)) {
+        if (Object.keys(result).indexOf(key) !== -1) {
             result = result[key];
             prefix = '';
         } else {

diff --git a/helpers/moment.js b/helpers/moment.js
@@ -53,7 +53,7 @@ const factory = () => {
             var res = moment(str);
             for (var key in options.hash) {
                 // prevent access to prototype methods
-                if (res.hasOwnProperty(key) && typeof res[key] === 'function') {
+                if (Object.keys(moment.prototype).indexOf(key) !== -1 && typeof res[key] === 'function') {
                     return res[key](options.hash[key]);
                 } else {
                     console.error('moment.js does not support "' + key + '"');

diff --git a/helpers/thirdParty.js b/helpers/thirdParty.js
@@ -95,6 +95,8 @@ const whitelist = [
             'extend',
             'forIn',
             'forOwn',
+            'get', // to be replaced
+            'getObject', // to be replaced
             'toPath',
             'hasOwn',
             'isObject',

diff --git a/spec/helpers.js b/spec/helpers.js
@@ -181,9 +181,9 @@ describe('helper registration', () => {
             'equals',
             'getShortMonth',
             'pick',
-        ];
+        ].sort();
 
-        expect(helpers.map(helper => helper.name)).to.be.equal(expectedHelpers);
+        expect(helpers.map(helper => helper.name).sort()).to.be.equal(expectedHelpers);
         done();
     });
 });
diff --git a/spec/helpers/get.js b/spec/helpers/get.js
@@ -23,25 +23,70 @@ describe('get helper', function () {
         ], done);
     });
 
-    it('does not access prototype properties', (done) => {
-        context.__proto__ = {x: 'yz'};
+    // uncomment when 3rd-party version is replaced
+    // it('does not access prototype properties', (done) => {
+    //     context.__proto__ = {x: 'yz'};
+    //     runTestCases([
+    //         {
+    //             input: `{{get "x" this}}`,
+    //             output: ``,
+    //         }
+    //     ], done);
+    // });
+
+    it('accepts SafeString paths', (done) => {
+        runTestCases([
+            {
+                input: `{{get (concat 'a' 'a') this}}`,
+                output: `a`,
+            },
+            {
+                input: `{{get (concat 'a' 'b') this}}`,
+                output: `b`,
+            },
+            // uncomment when 3rd-party version is replaced
+            // {
+            //     input: `{{get (concat 'options.a' '.b.c') this}}`,
+            //     output: `d`,
+            // }
+        ], done);
+    });
+
+    it('gets context from options object if none was given', (done) => {
         runTestCases([
             {
-                input: `{{get "x" this}}`,
+                input: `{{#get 'hash.a' a='some string'}}{{this}}{{/get}}`,
+                output: `some string`,
+            },
+        ], done);
+    });
+
+    it('renders to the empty string if no args are passed', (done) => {
+        runTestCases([
+            {
+                input: `{{get }}`,
                 output: ``,
             }
         ], done);
     });
 
-    it('accepts SafeString paths', (done) => {
+    it('returns the object arg if it is a string, number, undefined, or null', (done) => {
         runTestCases([
             {
-                input: `{{get (concat 'a' 'a') this}}`,
-                output: `a`,
+                input: `{{get 'a' 'some string'}}`,
+                output: `some string`,
             },
             {
-                input: `{{get (concat 'a' 'b') this}}`,
-                output: `b`,
+                input: `{{get 'a' 42}}`,
+                output: `42`,
+            },
+            {
+                input: `{{get 'a' undefined}}`,
+                output: ``,
+            },
+            {
+                input: `{{get 'a' null}}`,
+                output: ``,
             }
         ], done);
     });

diff --git a/spec/helpers/getObject.js b/spec/helpers/getObject.js
@@ -33,15 +33,16 @@ describe('getObject helper', function () {
         ], done);
     });
 
-    it('does not access prototype props', function (done) {
-        context.obj.__proto__ = {x: 'yz'};
-        runTestCases([
-            {
-                input: `{{#with (getObject "x" obj)}}{{x}}{{/with}}`,
-                output: ``,
-            },
-        ], done);
-    });
+    // uncomment when 3rd-party version is replaced
+    // it('does not access prototype props', function (done) {
+    //     context.obj.__proto__ = {x: 'yz'};
+    //     runTestCases([
+    //         {
+    //             input: `{{#with (getObject "x" obj)}}{{x}}{{/with}}`,
+    //             output: ``,
+    //         },
+    //     ], done);
+    // });
 
     it('accepts SafeString paths', (done) => {
         runTestCases([

diff --git a/spec/helpers/lib/common.js b/spec/helpers/lib/common.js
@@ -51,8 +51,8 @@ describe('common utils', function () {
         });
 
         it('should return obj[String(path)] or undefined if path is not a string or array', (done) => {
-            expect(getValue(globals, obj, {})).to.equal(undefined);
-            expect(getValue(globals, obj)).to.equal(undefined);
+            expect(getValue(globals, obj, {a: 1})).to.equal(undefined);
+            expect(getValue(globals, obj, ()=>1)).to.equal(undefined);
             expect(getValue(globals, obj, 42)).to.equal(42);
             done();
         });
@@ -62,9 +62,10 @@ describe('common utils', function () {
             done();
         });
 
-        it('should get empty string key if path is \'\'', (done) => {
-            expect(getValue(globals, obj, '')).to.equal(undefined);
-            expect(getValue(globals, {'': 0}, '')).to.equal(0);
+        it('should return obj if path is falsey', (done) => {
+            expect(getValue(globals, obj, '')).to.equal(obj);
+            expect(getValue(globals, obj, false)).to.equal(obj);
+            expect(getValue(globals, obj, 0)).to.equal(obj);
             done();
         });
 

diff --git a/spec/helpers/moment.js b/spec/helpers/moment.js
@@ -11,9 +11,18 @@ describe('moment helper', function () {
         const now = new Date();
         runTestCases([
             {
-                input: `{{#moment "1 year ago" "YYYY"}}{{/moment}}`,
+                input: `{{moment "1 year ago" "YYYY"}}`,
                 output: `${now.getFullYear() - 1}`,
             },
         ], done);
     });
+
+    it('permits use of moment.js functions', (done) => {
+        runTestCases([
+            {
+                input: `{{moment "2022-01-01" isAfter="1999-12-31"}}`,
+                output: `true`,
+            }
+        ], done);
+    });
 });