feat: STRF-9707 Improve money helper to support input params #164
Conversation
spec/helpers/strReplace.js
Outdated
| @@ -56,6 +56,7 @@ describe('strReplace helper', function() { | |||
|
|
|||
| it('should throw an exception if the parameters have an invalid type', function(done) { | |||
| renderString('{{strReplace object "none" "Bob"}}').catch(e => { | |||
| // console.log(e); | |||
There was a problem hiding this comment.
do we need this comment here?
There was a problem hiding this comment.
no, I thought I removed it :)
| if (!_.isNumber(value)) { | ||
| return ''; | ||
| // remove options hash object | ||
| args.pop(); |
There was a problem hiding this comment.
Are we able to exclude this by doing something like return function(options, ...args) to pull out the first param as "options" and just use the remaining in args to avoid this step?
There was a problem hiding this comment.
not really, cause I'm removing the last parameter, rather then the first one.
So it should be like return function(...args, options), but this kind of syntax is not supported 😒
There was a problem hiding this comment.
is it necessary to remove this at all, since we're just accessing the other elements by args[0] etc, wont those indexes be the same regardless if the last element in present?
There was a problem hiding this comment.
@jmwiese the thing is that number of arguments may vary. Just to be sure, that hash object (that has handlebars utility functions) was removed, I'm using a pop(). We are also using same approach in other helpers, to actually read that hash object by assigning it to a variable.
What? Why?
Improved money helper to support input params according to the docs
How was it tested?
npm test, e2ecc @bigcommerce/storefront-team