Bumped ast-types version

[ethancrook99]

No description provided.

[ethancrook99]

I'm not sure how to fix the failing test... I looked through the code, but I couldn't identify what change I needed to make to handle the new types. Any pointers?

[eventualbuddha]

My guess is that the new ast-types adds a new node type that recast doesn't know about?

[ethancrook99]

@eventualbuddha It's the new ChainElement and ChainExpression types, I'm just not sure how to make recast recognize new types

[mjfaga]

I updated the test to work past this here: #769

[benjamn]

@ethancrook99 The reason these tests exist is to ensure that the Recast pretty-printer is able to print the new AST types, so the conservative pretty-printing algorithm can fall back to generating valid (if somewhat reformatted) code wherever the original AST has been modified.

Where the AST has not been modified, Recast is able to copy the original code directly to the output (give or take some reindentation). In other words, Recast can often get away with not actually knowing how to print unchanged AST nodes, but it will fail badly when AST nodes involving a ChainExpression are modified or added to the AST, because it can't just copy the original code in those cases.

If you need [email protected] specifically because you want to write AST transforms using Recast that involve ChainExpression AST nodes, then you will almost certainly run into cases where you need Recast to be able to print these nodes, so I trust you can understand why simply adding the new types to the switch statement without actually implementing them (like #769 does) would be side-stepping the issue.

In short, the test failures are telling us that we need to add printer cases to lib/printer.ts (where you can find lots of examples of similar code). I can tackle that soon, unless you want to give it a try?

[ethancrook99]

@benjamn I can give it a shot! I'll let you know if I need any guidance

[ethancrook99]

I added possible implementations for the new types in printer.ts, but I feel like I may have oversimplified ChainExpression. I just copied directly from ExpressionStatement--should I have done something different here?

[eventualbuddha]

Close, should be easy to fix.

[mjfaga]

Thanks for putting in the remaining effort on this. I hadn't had a chance to review the patterns in play just yet and noticed the TODOs in there and followed that in the short term to resolve an issue I ran into locally on this very thing which did not require printing the Optional Chain operator.

Apologies for the side-step @benjamn 👍

[eventualbuddha]

Wasn't there a test? Where'd that go?

[ethancrook99]

I never actually added the test, I couldn't figure out how to get it working properly. I put the code that I did have in my reply to the above thread though, if you have any suggestions on how to fix it then I'll add the test back in

[ethancrook99]

Can I get some guidance on how to make a test for "a(b?.c)" not printing semicolons? I can't quite figure out how to follow the same pattern as the other printer tests.

[eventualbuddha]

I wouldn't worry about the semicolon thing specifically. Just have a test that generically prints a chain element.

[ethancrook99]

@eventualbuddha I added a couple of tests, looks like everything is working together nicely now

[benjamn]

@ethancrook99 @eventualbuddha Just published these changes to npm as [email protected], with some additional tests and tweaks in #783. Thanks for working on this! 🙌