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

Fix infinite recursion when printing sub-nodes #671

Conversation

courajs
Copy link
Contributor

@courajs courajs commented Nov 5, 2021

Built starting from #653, so that should be merged first. For an easier code review, look at the diff against that branch
Fixes #586
Includes test from #586, with some expanded coverage and a different fix.

@courajs courajs changed the title Fix 586 infinite recursion printing subnodes Fix infinite recursion when printing sub-nodes Nov 5, 2021
MichalBryxi and others added 6 commits November 10, 2021 07:21
- Test proving that when you reuse parts from previous concat statement in new concat statement you will get "Maximum callstack size exceeded"
…verflow

- Seems like anything that comes out of `parse()` apart from the whole object will trigger the stack overflow.
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.
@courajs courajs force-pushed the fix-586-infinite-recursion-printing-subnodes branch from 767302c to d9d9f5d Compare November 10, 2021 12:22
@courajs courajs marked this pull request as ready for review November 10, 2021 12:24
@dcyriller dcyriller added the bug Something isn't working label Nov 10, 2021
@MichalBryxi
Copy link
Contributor

@courajs Seems like this one got approved. Can we have a merge?

@courajs
Copy link
Contributor Author

courajs commented Dec 8, 2021

@courajs Seems like this one got approved. Can we have a merge?

A good question for someone with write access :)
@rwjblue @dcyriller

@dcyriller dcyriller merged commit 8b314e7 into ember-template-lint:master Dec 8, 2021
@dcyriller
Copy link
Member

Thanks a lot both of you!

@courajs courajs deleted the fix-586-infinite-recursion-printing-subnodes branch December 8, 2021 23:11
@dcyriller
Copy link
Member

It is released in v6.1.1

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

Successfully merging this pull request may close these issues.

Maximum call stack size exceeded
3 participants