From f4652af33eb039e610d6715364be0401a7423262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Bryxi=CC=81?= Date: Wed, 9 Jun 2021 22:18:18 +0100 Subject: [PATCH 1/6] test(#586): can reuse previous concat parts - Test proving that when you reuse parts from previous concat statement in new concat statement you will get "Maximum callstack size exceeded" --- src/parse-result.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/parse-result.test.ts b/src/parse-result.test.ts index a800ae5c..74ae4222 100644 --- a/src/parse-result.test.ts +++ b/src/parse-result.test.ts @@ -27,6 +27,18 @@ describe('ember-template-recast', function () { expect(print(ast)).toEqual(``); }); + test('can reuse previous concat parts', function () { + let template = ``; + let original = parse(template); + let element = original.body[0] as AST.ElementNode; + let attribute = element.attributes[0] as AST.AttrNode; + let concat = attribute.value as AST.ConcatStatement; + + let ast = builders.concat([concat.parts[0], concat.parts[1]]); + + expect(print(ast)).toEqual(``); + }); + test('changing an element to a void element does not print closing tag', function () { let template = `
`; From ed14164415f3342a3e3ac0dadd97510168acfffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Bryxi=CC=81?= Date: Thu, 10 Jun 2021 11:27:44 +0100 Subject: [PATCH 2/6] test(#586): simplified version of test for stack overflow - Seems like anything that comes out of `parse()` apart from the whole object will trigger the stack overflow. --- src/parse-result.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/parse-result.test.ts b/src/parse-result.test.ts index 74ae4222..1a371ae2 100644 --- a/src/parse-result.test.ts +++ b/src/parse-result.test.ts @@ -39,6 +39,14 @@ describe('ember-template-recast', function () { expect(print(ast)).toEqual(``); }); + test('can print parsed element', function () { + let template = ``; + let original = parse(template); + let element = original.body[0] as AST.ElementNode; + + expect(print(element)).toEqual(``); + }); + test('changing an element to a void element does not print closing tag', function () { let template = `
`; From 0d0774612ec72d20d6754aa903aa6846455a9d6b Mon Sep 17 00:00:00 2001 From: Cyrille David Date: Thu, 4 Nov 2021 01:39:03 +0100 Subject: [PATCH 3/6] Keep the simpler test from the two --- src/parse-result.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/parse-result.test.ts b/src/parse-result.test.ts index 1a371ae2..f9fe3e4d 100644 --- a/src/parse-result.test.ts +++ b/src/parse-result.test.ts @@ -27,18 +27,6 @@ describe('ember-template-recast', function () { expect(print(ast)).toEqual(``); }); - test('can reuse previous concat parts', function () { - let template = ``; - let original = parse(template); - let element = original.body[0] as AST.ElementNode; - let attribute = element.attributes[0] as AST.AttrNode; - let concat = attribute.value as AST.ConcatStatement; - - let ast = builders.concat([concat.parts[0], concat.parts[1]]); - - expect(print(ast)).toEqual(``); - }); - test('can print parsed element', function () { let template = ``; let original = parse(template); From f3e61cc41166aefa12163cd9f6bc43ee765f1b6f Mon Sep 17 00:00:00 2001 From: Cyrille David Date: Thu, 4 Nov 2021 01:58:15 +0100 Subject: [PATCH 4/6] Wrap the ast to print in a template (with builders.template) --- src/parse-result.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/parse-result.test.ts b/src/parse-result.test.ts index f9fe3e4d..74d453e4 100644 --- a/src/parse-result.test.ts +++ b/src/parse-result.test.ts @@ -27,12 +27,13 @@ describe('ember-template-recast', function () { expect(print(ast)).toEqual(``); }); - test('can print parsed element', function () { - let template = ``; + test('reusing another template part to build a new template', function () { + let template = `foo`; let original = parse(template); - let element = original.body[0] as AST.ElementNode; + let text = original.body[0] as AST.TextNode; + let ast = builders.template([text]); - expect(print(element)).toEqual(``); + expect(print(ast)).toEqual(`foo`); }); test('changing an element to a void element does not print closing tag', function () { From 338a2ce3b31c7a2a62dd27cb4b3cea8823333488 Mon Sep 17 00:00:00 2001 From: Cyrille David Date: Thu, 4 Nov 2021 02:00:31 +0100 Subject: [PATCH 5/6] Do not try to reuse a parsed node --- src/index.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/index.ts b/src/index.ts index 92b2f87e..69556f65 100644 --- a/src/index.ts +++ b/src/index.ts @@ -19,14 +19,7 @@ export function print(ast: AST.Node): string { // TODO: write a test for this case if (parseResult === undefined) { - return glimmerPrint(ast, { - entityEncoding: 'raw', - override: (ast) => { - if (NODE_INFO.has(ast)) { - return print(ast); - } - }, - }); + return glimmerPrint(ast, { entityEncoding: 'raw' }); } return parseResult.print(); From d9d9f5d04f04bd52978428ad4c1a864a36301086 Mon Sep 17 00:00:00 2001 From: Aaron Sikes Date: Fri, 5 Nov 2021 17:46:22 -0400 Subject: [PATCH 6/6] Make printing more robust to manually created glimmer nodes Fixes https://github.com/ember-template-lint/ember-template-recast/issues/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. --- src/index.ts | 24 ++++++++++-------------- src/parse-result.test.ts | 17 +++++++++++++++++ src/parse-result.ts | 2 ++ 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/index.ts b/src/index.ts index 69556f65..7cc072b3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -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(); const NODE_INFO = new WeakMap(); 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 { diff --git a/src/parse-result.test.ts b/src/parse-result.test.ts index 74d453e4..43ec7cb4 100644 --- a/src/parse-result.test.ts +++ b/src/parse-result.test.ts @@ -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 = ``; + let original = parse(template); + + let raw_wrapping_ast = builders.template([ + builders.element('div', { + children: original.body, + }), + ]); + + expect(print(raw_wrapping_ast)).toEqual(`
`); + }); + test('changing an element to a void element does not print closing tag', function () { let template = `
`; diff --git a/src/parse-result.ts b/src/parse-result.ts index bf07ad03..a2258cb5 100644 --- a/src/parse-result.ts +++ b/src/parse-result.ts @@ -115,6 +115,7 @@ export interface NodeInfo { node: AST.Node; original: AST.Node; source: string; + parse_result: ParseResult; hadHash?: boolean; hadParams?: boolean; @@ -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);