Closed bpstrngr closed 11 months ago
small update: found Acorn has a "preserveParens" option, but that just inserts Nodes that Astring doesn't recognise to begin with. Also found a related pack of issues, but those produce runtime bugs, not syntactic errors: https://github.com/estree/estree/issues/194
Thanks for raising this @bpstrngr. Will fix it this week unless someone else does sooner.
Hi @bpstrngr, unless I'm missing something, this issue couldn't be reproduced. It's actually covered by tests here: https://github.com/davidbonnet/astring/blob/8ac784be966a20eda44de32f79db1a6e44832659/src/tests/fixtures/syntax/precedence.js#L11
Could be that your project isn't using the latest version of astring
.
@davidbonnet i moved past this since, i'm not sure what dissipated the problem. Could be that i didn't check my version, sorry. Thank you for looking into it!
Motivation
I'm building a loader module that performs transformations on AST-s and re-serializes them with Astring.
Expected behavior
A function declaration in Rollup's src/utils/options/normalizeOutputOptions.js file reading
contains a LogicalExpression node that Acorn parses as the following:
which should be reversible to the valid expression with Astring.
Actual behavior
Passing the unmodified AST back to Astring produces the following string:
which, passed further in my loader hook fails with this error:
I was surprised that this is an invalid syntax, but as i confirmed in repl it's only valid with brackets around the left side:
Is this a problem with Acorn (its plugins can interfere with the produced AST), or the serializer?
Either Acorn should preserve some "bracket" parent node above the left side (being in the business of transforming AST-s i could handle that, but I never know where exactly to look up how that Node would look like form Acorn-adjacent docs), or Astring should detect the need for brackets in this situation.
I'm using Astring v1.8.6.