String replace helper with more options #156
Conversation
helpers/strReplace.js
Outdated
|
|
||
| const factory = () => { | ||
| return function(str, substr, newSubstr, iteration) { | ||
| if (typeof str !== 'string' || typeof substr !== 'string' || typeof newSubstr !== 'string') { |
There was a problem hiding this comment.
In case you want to use this helper with concat or replace helper this condition won't pass.
To unlock that possibility, please use https://github.com/bigcommerce/paper-handlebars/blob/master/helpers/lib/common.js#L12 before type checking.
helpers/strReplace.js
Outdated
| const factory = () => { | ||
| return function(str, substr, newSubstr, iteration) { | ||
| if (typeof str !== 'string' || typeof substr !== 'string' || typeof newSubstr !== 'string') { | ||
| return 'Invalid Input'; |
There was a problem hiding this comment.
I think you would rather want to see here an Error, rather than a "Invalid Input" line.
Please take a look at TypeError and maybe passing some arguments there, so it will be easier to debug
|
@jairo-bc Thanks! |
spec/helpers/strReplace.js
Outdated
| runTestCases([ | ||
| { | ||
| input: '{{strReplace 5 5 5}}', | ||
| output: 'Invalid Input', |
There was a problem hiding this comment.
I think this test case also should be reworked to accommodate errors throwing
There was a problem hiding this comment.
Yes sir! Updated test case in the new commit.
spec/helpers/strReplace.js
Outdated
| ], done); | ||
| }); | ||
|
|
||
| it('should only handle string', function(done) { |
There was a problem hiding this comment.
Please change the test name to more meaningful description to what it doest
| }); | ||
|
|
||
| it('should only handle string', function(done) { | ||
| renderString('{{strReplace object "none" "Bob"}}').catch(e => { |
There was a problem hiding this comment.
I would suggest you to use this kind of syntax https://hapi.dev/module/code/api?v=8.0.6#throwtype-message to catch errors and match the errors message expectations
There was a problem hiding this comment.
@jairo-bc Hi! I have updated the wording for the description. For catching errors, I am using the same format as other helpers, can you give me more details about how I can improve this please? Thank you!
What? Why?
This is an inline string replacement helper that gives the user more control in how many occurrences of the substring to be replaced.
How was it tested?
It was tested by giving several test cases including inputs that are out of bound.
{{strReplace string substr newSubstr}}would replace all substring by default{{strReplace string substr newSubstr count}}would replace count of occurrences.Invalid string input would return invalid.
Invalid count would return original string and extra count (count > occurrences) would regard as replace all.
cc @bigcommerce/storefront-team