htmlbars icon indicating copy to clipboard operation
htmlbars copied to clipboard

HTMLBars AST does not push CommentStatements?

Open step2yeung opened this issue 9 years ago • 3 comments

I noticed that HTMLBars AST does not push mustache statement styled comments into the AST, while HandleBars AST does. Is there a reason for this difference?

For example:

{{! comments }}
<div">
  foobar
</div>

Handlebars AST looks like (Notice the CommentStatement element):

{ loc: 
   { source: undefined,
     start: { line: 1, column: 0 },
     end: { line: 5, column: 0 } },
  type: 'Program',
  body: 
   [ { loc: [Object],
       type: 'CommentStatement',
       value: ' comments ',
       strip: [Object] },
     { loc: [Object],
       type: 'ContentStatement',
       value: '<div class="panel">\n  foobar\n</div>\n',
       original: '\n<div class="panel">\n  foobar\n</div>\n',
       rightStripped: true } ],
  blockParams: null,
  strip: {} }

While, HTMLBar AST does not push the comment statement into the AST elements.

{ type: 'Program',
  body: 
   [ { type: 'ElementNode',
       tag: 'div',
       attributes: [Object],
       modifiers: [],
       children: [Object],
       loc: [Object] },
     { type: 'TextNode', chars: '\n', loc: null } ],
  blockParams: [],
  loc: 
   { source: null,
     start: { line: 1, column: 0 },
     end: { line: 5, column: 0 } } }

step2yeung avatar Sep 28 '15 21:09 step2yeung

This is a bug. We need to preserve both HTML and HBS comments.

mmun avatar Sep 28 '15 22:09 mmun

The more I think about this it actually seems like not a bug. If the objective of HTMLBars' parsing is to produce an AST that ultimately maps to DOM, then preserving Handlebars comments has no meaningful impact on the DOM, unlike HTML comments.

zackthehuman avatar Dec 18 '15 17:12 zackthehuman

Yes, they are 100% inert but for practical reasons it would be useful to have them in the AST, e.g. for future linting tools. We may opt instead to have a separate Concrete Syntax Tree but my hunch is that there isn't enough differences between the AST and CST to merit the extra complexity.

mmun avatar Dec 18 '15 18:12 mmun