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

Maximum call stack size exceeded #586

Closed
MichalBryxi opened this issue Jun 9, 2021 · 5 comments · Fixed by #671
Closed

Maximum call stack size exceeded #586

MichalBryxi opened this issue Jun 9, 2021 · 5 comments · Fixed by #671
Labels
bug Something isn't working

Comments

@MichalBryxi
Copy link
Contributor

Somewhere between v5.0.1 and v5.0.3 we got an issue that manifests as follows (log taken from ember-template-lint-plugin-tailwindcss):

    +     "message": "Maximum call stack size exceeded",
    +     "severity": 2,
    +     "source": "RangeError: Maximum call stack size exceeded
    +     at Object.get [as print] (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/node_modules/@glimmer/syntax/dist/commonjs/es2017/index.js:78:17)
    +     at print (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/src/index.ts:21:12)
    +     at Object.override (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/src/index.ts:25:18)
    +     at Printer.print (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/node_modules/@glimmer/syntax/packages/@glimmer/syntax/lib/generation/printer.ts:539:20)
    +     at Object.build (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/node_modules/@glimmer/syntax/packages/@glimmer/syntax/lib/generation/print.ts:13:10)
    +     at print (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/src/index.ts:21:12)
    +     at Object.override (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/src/index.ts:25:18)
    +     at Printer.print (/Users/michal/ember/ember-template-lint-plugin-tailwindcss/node_modules/ember-template-recast/node_modules/@glimmer/syntax/packages/@glimmer/syntax/lib/generation/printer.ts:539:20)
@rwjblue
Copy link
Member

rwjblue commented Jun 9, 2021

Do you happen to know the steps to repro? Our internal tests don't seem to hit this, it's going to be hard to figure out what is wrong without some high level understanding of what manipulation causes it... 🤔

Maybe you could copy a test like this one, to replicate?

test('updating attributes on a non-self-closing void element', function () {
let template = `<img src="{{something}}">`;
let ast = parse(template);
let element = ast.body[0] as AST.ElementNode;
let attribute = element.attributes[0] as AST.AttrNode;
let concat = attribute.value as AST.ConcatStatement;
(concat.parts[0] as AST.MustacheStatement).path = builders.path('this.something');
expect(print(ast)).toEqual(`<img src="{{this.something}}">`);
});

@MichalBryxi
Copy link
Contributor Author

No idea yet. It might even be a complete red herring as only when I shake the yarn.lock I get the problematic behaviour. I will try to get a failing test for you.

@MichalBryxi
Copy link
Contributor Author

MichalBryxi commented Jun 9, 2021

I could trace it to this recursive print() function. When I try to console.log(ast) for my case that trips over, I get something like:

{
        type: 'ConcatStatement',
        parts: [
          {
            type: 'TextNode',
            chars: '\n' +
              '      bazDoo\n' +
              '      gap-4 grid grid-cols-2\n' +
              '      lg:grid-cols-5 md:grid-cols-3 sm:grid-cols-3 xl:grid-cols-6\n' +
              '      ',
            loc: [Object]
          },
          {
            type: 'MustacheStatement',
            path: [Object],
            params: [Array],
            hash: [Object],
            escaped: true,
            loc: [Object],
            strip: [Object]
          },
          { type: 'TextNode', chars: '\n      ', loc: [Object] },
          {
            type: 'MustacheStatement',
            path: [Object],
            params: [Array],
            hash: [Object],
            escaped: true,
            loc: [Object],
            strip: [Object]
          },
          { type: 'TextNode', chars: '\n    ', loc: [Object] }
        ],
        loc: {
          source: null,
          start: { line: 3, column: 10 },
          end: { line: 13, column: 5 }
        }
}

Which seems reasonable to me. But I don't understand why it's recursing infinitely nor what's the logic of that function in the first place.

MichalBryxi added a commit to MichalBryxi/ember-template-recast that referenced this issue Jun 9, 2021
- Test proving that when you reuse parts from previous concat statement in new concat statement you will get "Maximum callstack size exceeded"
MichalBryxi added a commit to MichalBryxi/ember-template-recast that referenced this issue Jun 9, 2021
- Test proving that when you reuse parts from previous concat statement in new concat statement you will get "Maximum callstack size exceeded"
@MichalBryxi
Copy link
Contributor Author

I think I found a repro: #589

Seems like when I create new concat with parts from previous concat bad things will happen. cc: @rwjblue

MichalBryxi added a commit to MichalBryxi/ember-template-recast that referenced this issue Jun 10, 2021
…verflow

- Seems like anything that comes out of `parse()` apart from the whole object will trigger the stack overflow.
@dcyriller dcyriller added the bug Something isn't working label Nov 4, 2021
courajs added a commit to courajs/ember-template-recast that referenced this issue Nov 5, 2021
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.
dcyriller pushed a commit that referenced this issue Nov 10, 2021
- Test proving that when you reuse parts from previous concat statement in new concat statement you will get "Maximum callstack size exceeded"
dcyriller pushed a commit that referenced this issue Nov 10, 2021
- Seems like anything that comes out of `parse()` apart from the whole object will trigger the stack overflow.
courajs pushed a commit to courajs/ember-template-recast that referenced this issue Nov 10, 2021
- Test proving that when you reuse parts from previous concat statement in new concat statement you will get "Maximum callstack size exceeded"
courajs pushed a commit to courajs/ember-template-recast that referenced this issue Nov 10, 2021
…verflow

- Seems like anything that comes out of `parse()` apart from the whole object will trigger the stack overflow.
courajs added a commit to courajs/ember-template-recast that referenced this issue Nov 10, 2021
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.
@MichalBryxi
Copy link
Contributor Author

Sounds like this one was resolved with #671

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 a pull request may close this issue.

3 participants