Description
I am using Pattern Lab Node v1.2.0
on Mac
, using the gulp
configuration.
Expected Behavior
I have a mustache file which contains a pattern include with 3 parameters, the first two of which have non-string values (as they are booleans): {{> molecules-product-search-actions( large: true, useDefaultStyle: true, prodButtonLabel: "View product" ) }}
.
I expect Pattern Lab to parse those parameters correctly and use them in the included partial.
Actual Behavior
When I run Pattern Lab node, the build displays warning messages along the lines of [SyntaxError: Unexpected token u]
, but appears to complete nonetheless.
When I view the generated page, the values I have passed to the partial via the pattern parameters appear to have been lost. The included partial renders as if all of its data values have been unset.
Steps to Reproduce
You can simulate this using the out-of-the box mustache templates that ship with Pattern Lab like so:
- Edit
source/_patterns/02-organisms/02-comments/01-sticky-comment.mustache
and add a parameter with a non-quoted value to one of the partial includes in that file. For example:{{> molecules-single-comment(test: 42, description: 'We are all in the gutter, ....') }}
- Re-build by running
grunt
(orgulp
)- Note that you'll see a few
[SyntaxError: Unexpected token d]
messages on the console during the build.
- Note that you'll see a few
- Open the generated site in a web browser and navigated to "Pages > Article"
- Scroll down the page to comments section and you'll see that the text and avatar image for the comment you edited have disappeared.
Root cause and potential fix
I did a bit of investigating and traced this bug back to the logic inside parameter_hunter.js
. The paramToJson()
function is incorrectly advancing the delimitPos
by 1 for cases where the param value is not wrapped in quotes. This means the the comma after that param is removed from the remaining paramString
, which gets processed on the next loop iteration.
Since the two regexes at the beginning of the loop expect either an opening curly brace or comma at the beginning of paramString
they don't match. Therefore the code incorrectly assumes it has double-quoted param key and appends it to the output as is.
The net result is that paramToJson()
returns a string with an unquoted key, which in turn generates the [SyntaxError: Unexpected token d]
when findparameters()
passes it into JSON.parse()
. That then screws up the remaining logic and, well, you get the idea :-)
So, a simple fix might be to simple remove lines 105 - 107 so that delimitPos
isn't moved on when it shouldn't be:
...
//except to prevent infinite loops.
if (delimitPos === -1) {
delimitPos = paramString.length - 1;
}
// FIX:
//else {
// delimitPos += 1;
//}
paramStringWellFormed += paramString.substring(0, delimitPos);
...
However, while looking at this code, I spotted a number of other bugs lurking:
- Quoted key that contain escaped quotes within them (e.g.
'silly param \' key'
) cause similar issues because the regexes in the loop don't handle escaped quotes. - Similarly, quoted string values that contain a closing parenthesis
)
confusefindparameters()
as it incorrectly treats that as the end of the pattern's parameters - Maybe more
I therefore set about trying to fix all of these issues in a fork. I went a tad overboard and ended up replacing all of paramToJson()
. However, I believe I have managed to make it more robust. In addition to fixing the above bugs, it now handles incomplete key : value pairs more gracefully too.
I will add a few more test cases before creating a pull request, but if you're curious you can take a peak via the link above.