Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly escape string literals and attribute values #698

Open
courajs opened this issue Nov 29, 2021 · 1 comment
Open

Properly escape string literals and attribute values #698

courajs opened this issue Nov 29, 2021 · 1 comment
Labels
bug Something isn't working

Comments

@courajs
Copy link
Contributor

courajs commented Nov 29, 2021

Thanks to this comment from @bmish, I realized that the recently merged #653 can produce invalid html when changing quoteType. Looking into the issue more, I see there was an already existing issue that changing the value of string literals or AttrNodes can produce invalid html.

But, I'm kinda tired of chasing these kinds of things down.
In my opinion, we need a proper, well-typed Glimmer concrete syntax tree, parser, and printer. I'm thinking of writing one, but probably won't be whacking any more moles like this in the meantime.

Here's the relevant section on html attribute syntax: https://html.spec.whatwg.org/dev/syntax.html#attributes-2
Here are some tests that could be added to parse-result.test.ts, if someone else would like to fix these issues here:

    test('Escape quotes properly when changing string values', function () {
      let ast = parse(`{{helper 'val'}}`) as any;
      ast.body[0].params[0].value = "aaron's house";
      expect(print(ast)).toEqual(``);
    });
    test('Escape string literals when changing quote type', function () {
      let ast = parse(`{{helper "aaron's house"}}`) as any;
      ast.body[0].params[0].quoteType = "'";
      expect(print(ast)).toEqual(`{{helper 'aaron\\'s house'}}`);

      ast = parse(`{{helper '"so-called"'}}`);
      ast.body[0].params[0].quoteType = '"';
      expect(print(ast)).toEqual(`{{helper '\\"so-called\\"'}}`);
    });

    test('Escape attribute values when changing quote type', function () {
      let ast = parse(`<img alt="john's house">`) as any;
      ast.body[0].attributes[0].quoteType = "'";
      expect(print(ast)).toEqual(`<img alt='john\\'s house'>`);

      ast = parse(`<img alt='"so-called"'>`);
      ast.body[0].attributes[0].quoteType = '"';
      expect(print(ast)).toEqual(`<img alt="\\"so-called\\"">`);

      ast = parse(`<div class="john's house {{this.otherClass}}"></div>`);
      ast.body[0].attributes[0].quoteType = "'";
      expect(print(ast)).toEqual(`<div class='john&apos;s house {{this.otherClass}}"></div>`);

      ast = parse(`<div class='a "so-called" btn {{this.otherClass}}'></div>`);
      ast.body[0].attributes[0].quoteType = '"';
      expect(print(ast)).toEqual(`<div class="a &quot;so-called&quot; btn {{this.otherClass}}"></div>`);
    });
@robclancy
Copy link

These rules are generally following prettier it seems and what it would do is simply leave it as is. If a quote is in a string in prettier it uses the opposite type to wrap it regardless of the setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants