Skip to content

Pattern parameters with non-string values cause the param values to disappear #291

Closed
@james-nash

Description

@james-nash

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:

  1. 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, ....') }}
  2. Re-build by running grunt (or gulp)
    • Note that you'll see a few [SyntaxError: Unexpected token d] messages on the console during the build.
  3. Open the generated site in a web browser and navigated to "Pages > Article"
  4. 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 ) confuse findparameters() 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions