Merge pull request #1266 from charlespierce/comment_attribute_order

Maintain relative order of attributes and comments within an `ElementNode`
glimmerjs · Feb 12, 2021 · 1a54cba · 1a54cba
2 parents ebd95ed + ba5e8a4
commit 1a54cba
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 18 deletions.
diff --git a/packages/@glimmer/syntax/index.ts b/packages/@glimmer/syntax/index.ts
@@ -14,6 +14,7 @@ export {
   PrecompileOptions,
 } from './lib/parser/tokenizer-event-handlers';
 export { default as print } from './lib/generation/print';
+export { sortByLoc } from './lib/generation/util';
 export { default as Walker } from './lib/traversal/walker';
 export { default as traverse } from './lib/traversal/traverse';
 export { NodeVisitor } from './lib/traversal/visitor';

diff --git a/packages/@glimmer/syntax/lib/generation/printer.ts b/packages/@glimmer/syntax/lib/generation/printer.ts
@@ -1,5 +1,5 @@
 import * as ASTv1 from '../v1/api';
-import { escapeAttrValue, escapeText } from './util';
+import { escapeAttrValue, escapeText, sortByLoc } from './util';
 
 export const voidMap: {
   [tagName: string]: boolean;
@@ -219,23 +219,21 @@ export default class Printer {
 
   OpenElementNode(el: ASTv1.ElementNode): void {
     this.buffer += `<${el.tag}`;
-    if (el.attributes.length) {
-      el.attributes.forEach((attr) => {
-        this.buffer += ' ';
-        this.AttrNode(attr);
-      });
-    }
-    if (el.modifiers.length) {
-      el.modifiers.forEach((mod) => {
-        this.buffer += ' ';
-        this.ElementModifierStatement(mod);
-      });
-    }
-    if (el.comments.length) {
-      el.comments.forEach((comment) => {
-        this.buffer += ' ';
-        this.MustacheCommentStatement(comment);
-      });
+    const parts = [...el.attributes, ...el.modifiers, ...el.comments].sort(sortByLoc);
+
+    for (const part of parts) {
+      this.buffer += ' ';
+      switch (part.type) {
+        case 'AttrNode':
+          this.AttrNode(part);
+          break;
+        case 'ElementModifierStatement':
+          this.ElementModifierStatement(part);
+          break;
+        case 'MustacheCommentStatement':
+          this.MustacheCommentStatement(part);
+          break;
+      }
     }
     if (el.blockParams.length) {
       this.BlockParams(el.blockParams);

diff --git a/packages/@glimmer/syntax/lib/generation/util.ts b/packages/@glimmer/syntax/lib/generation/util.ts
@@ -1,3 +1,5 @@
+import * as ASTv1 from '../v1/api';
+
 const enum Char {
   NBSP = 0xa0,
   QUOT = 0x22,
@@ -53,3 +55,30 @@ export function escapeText(text: string): string {
   }
   return text;
 }
+
+export function sortByLoc(a: ASTv1.Node, b: ASTv1.Node): -1 | 0 | 1 {
+  // If either is invisible, don't try to order them
+  if (a.loc.isInvisible || b.loc.isInvisible) {
+    return 0;
+  }
+
+  if (a.loc.startPosition.line < b.loc.startPosition.line) {
+    return -1;
+  }
+
+  if (
+    a.loc.startPosition.line === b.loc.startPosition.line &&
+    a.loc.startPosition.column < b.loc.startPosition.column
+  ) {
+    return -1;
+  }
+
+  if (
+    a.loc.startPosition.line === b.loc.startPosition.line &&
+    a.loc.startPosition.column === b.loc.startPosition.column
+  ) {
+    return 0;
+  }
+
+  return 1;
+}
diff --git a/packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts b/packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts
@@ -231,6 +231,7 @@ export abstract class HandlebarsNodeVisitors extends Parser {
 
     switch (tokenizer.state) {
       case TokenizerState.beforeAttributeName:
+      case TokenizerState.afterAttributeName:
         this.currentStartTag.comments.push(comment);
         break;
 

diff --git a/packages/@glimmer/syntax/test/generation/print-test.ts b/packages/@glimmer/syntax/test/generation/print-test.ts
@@ -55,6 +55,9 @@ let templates = [
 
   // unescaped
   '{{{unescaped}}}',
+
+  // Comment in Angle Bracket component
+  '<Foo {{!-- This is a comment --}} attribute></Foo>',
 ];
 
 QUnit.module('[glimmer-syntax] Code generation', function () {

diff --git a/packages/@glimmer/syntax/test/parser-node-test.ts b/packages/@glimmer/syntax/test/parser-node-test.ts
@@ -534,6 +534,16 @@ test('a Handlebars comment in proper element space', function () {
   );
 });
 
+test('a Handlebars comment after a valueless attribute', function () {
+  let t = '<input foo {{! comment }}>';
+  astEqual(
+    t,
+    b.program([
+      element('input', ['attrs', ['foo', '']], ['comments', b.mustacheComment(' comment ')]),
+    ])
+  );
+});
+
 test('a Handlebars comment in invalid element space', function (assert) {
   assert.throws(() => {
     parse('\nbefore <div \n  a{{! some comment }} data-foo="bar"></div> after', {