Use babel/parser as the default parser for formatting TypeScript

[sosukesuzuki]

Currently we are using typescript-estree as the default parser for TypeScript.

It is a great parser for linters, but it may not be a suitable parser for formatters in the future(ref: typescript-eslint/typescript-eslint#1852, typescript-eslint/typescript-eslint#2908 (comment))

Prettier sometimes formats broken (syntactically invalid) code using editor extensions and the like.

I personally think that the parser for formatters should not be strict.

So I think babel/parser with errorRecovery is enabled should be the default for TypeScript.

What do you think? /cc @prettier/core

Previous discussions:

• Always use Babylon for TypeScript and Flow #4103
• feat: add new parser babel-ts to parse TypeScript syntaxes via Babel #6400 (comment)

[bradzacher]

To clarify - for typescript-eslint/typescript-eslint#1852 I plan on adding an option to implement it, which would preserve the previous behaviour.

Prettier sometimes formats broken (syntactically invalid) code using editor extensions and the like.

I am curious - why does prettier do this?
Why not just fail and ignore syntactically invalid cases?

[sosukesuzuki]

I plan on adding an option to implement it, which would preserve the previous behaviour.

It would be great!

Why not just fail and ignore syntactically invalid cases?

On a text editor, we may have a use case that we want to format the unfinished code. It may be syntactically broken.

[bradzacher]

On a text editor, we may have a use case that we want to format the unfinished code. It may be syntactically broken.

That's what I'm curious about - why does prettier support incorrect / broken code?
Is there a past discussion so that I can read about the reasoning?

To me it seems like that just leads to trouble / danger and could lead to some weird states and unpredictable formatting. Also it leads to more maintenance burden for prettier as it has to ensure handle all of the invalid states.

EG from your example in typescript-eslint/typescript-eslint#2908

class Foo {
  readonly bar() {}
  declare baz() {}
}

Prettier has to understand that a user might write these (completely invalid) keywords, and ensure they're formatted appropriately.

[sosukesuzuki]

Is there a past discussion so that I can read about the reasoning?

I looked for the previous discussion but it not found, sorry.

Also it leads to more maintenance burden for prettier as it has to ensure handle all of the invalid states.

Prettier already has a lot of tests for invalid cases, so I don't think the maintenance burden will increase.

However, if the option which preserve the previous behaviour is added to ts-estree we don't need to switch default parser.

Anyway, I would like to know what other Prettier maintainers think about this.

[fisker]

I don't have strong opinion on default parser , but I don't like errorRecovery, I prefer throw error on invalid code.

[thorn0]

@fisker @bradzacher At some point Babel started to report errors ("duplicate identifier") for code like following:

const a = 2;
const a = 3;

So Prettier 1.19 was going to refuse to format it too, which felt too strict, especially compared to the TS parser and given that 1.18 formatted it without a problem. That's why using Babel's new error recovery mode seemed like a good idea. See also https://babeljs.io/blog/2019/11/05/7.7.0#parser-error-recovery-10363httpsgithubcombabelbabelpull10363

But yes, it also opened a can of worms: how far should Prettier go in ignoring parser errors? I personally would prefer if it hadn't gone any further than just ignoring those "duplicate identifier/property" errors.

[bradzacher]

I wonder if it's worth working with babel to introduce error codes so that you can use the error recovery mode to avoid the duplicate identifier/duplicate property errors without ignoring most syntax errors.

[thorn0]

Prettier already rethrows some recovered errors. Of course, having error codes would be much better than these hardcoded error messages,

[armano2]

So I think babel/parser with errorRecovery is enabled should be the default for TypeScript.

for now babel does not produce yet fully correct AST, I'm doing cross testing for ranges to make sure that its covering all edge cases

https://github.com/babel/babel/issues/created_by/armano2

one of examples that i recently discovered is JamesHenry/typescript-estree#109 - #5728 that also affects babel babel/babel#12825

i think good first step will be allowing users to actually use babel as typescript (ofc if its not a case already) and see if new issues will appear

[thorn0]

@armano2

ofc if its not a case already

It is. The "babel-ts" parser has been in Prettier for one year already. We are well aware about many of these AST format discrepancies including this one you mentioned. Prettier's TypeScript tests are all run against both parsers and verify that the output is the same.