Skip to content

Commit

Permalink
Make printing more robust to manually created glimmer nodes
Browse files Browse the repository at this point in the history
Fixes ember-template-lint#586

Previously, calling print on node that came from our parse(), but was
not the top-level node, would cause infinite recursion. (It would have
NodeInfo, but not a registered parse result).

This fixes the infinite loop, by just storing a reference to the
top-level parse result in the node info, and using that.

Importantly, this successfully preserves formatting for any sub-nodes we
did parse ourselves, or any user-created nodes that require custom
printing.
  • Loading branch information
courajs committed Nov 5, 2021
1 parent 2018a24 commit 767302c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 14 deletions.
24 changes: 10 additions & 14 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,22 @@ import type { ASTv1 as AST, NodeVisitor } from '@glimmer/syntax';
import ParseResult, { NodeInfo } from './parse-result';
import { builders } from './custom-nodes';

const PARSE_RESULT_FOR = new WeakMap<AST.Node, ParseResult>();
const NODE_INFO = new WeakMap<AST.Node, NodeInfo>();

export function parse(template: string): AST.Template {
const result = new ParseResult(template, NODE_INFO);

PARSE_RESULT_FOR.set(result.ast, result);

return result.ast;
return new ParseResult(template, NODE_INFO).ast;
}

export function print(ast: AST.Node): string {
const parseResult = PARSE_RESULT_FOR.get(ast);

// TODO: write a test for this case
if (parseResult === undefined) {
return glimmerPrint(ast, { entityEncoding: 'raw' });
}

return parseResult.print();
return glimmerPrint(ast, {
entityEncoding: 'raw',
override: (ast) => {
let info = NODE_INFO.get(ast);
if (info) {
return info.parse_result.print(ast);
}
},
});
}

export interface Syntax {
Expand Down
17 changes: 17 additions & 0 deletions src/parse-result.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@ describe('ember-template-recast', function () {
expect(print(ast)).toEqual(`foo`);
});

test('wrapping a parsed node (which uses custom formatting) with a raw node', function () {
// Ensuring fix for GH#586
// (infinite recursion when printing custom nodes containing parsed nodes)
// plays nicely with custom printing from GH#653
// (specifying quoteType on custom nodes, adds a printing override)
let template = `<Foo @class='a {{b}} c' />`;
let original = parse(template);

let raw_wrapping_ast = builders.template([
builders.element('div', {
children: original.body,
}),
]);

expect(print(raw_wrapping_ast)).toEqual(`<div><Foo @class='a {{b}} c' /></div>`);
});

test('changing an element to a void element does not print closing tag', function () {
let template = `<div data-foo="{{something}}"></div>`;

Expand Down
2 changes: 2 additions & 0 deletions src/parse-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export interface NodeInfo {
node: AST.Node;
original: AST.Node;
source: string;
parse_result: ParseResult;

hadHash?: boolean;
hadParams?: boolean;
Expand Down Expand Up @@ -156,6 +157,7 @@ export default class ParseResult {
node,
original: JSON.parse(JSON.stringify(node)),
source: this.sourceForLoc(node.loc),
parse_result: this,
};

this.nodeInfo.set(node, nodeInfo);
Expand Down

0 comments on commit 767302c

Please sign in to comment.