Remove flow support from parser: babel

[fisker]

I was trying to improve parse performance on large files, and found it will cost almost half time on some file by removing plugins.

Something interesting come up, we have flow and babel-flow parser, but we also enabled flow syntax in babel plugins https://github.com/prettier/prettier/blob/master/src/language-js/parser-babel.js#L101

Shouldn't we encourage people use flow or babel-flow? People use typescript and babel-ts for .ts, not babel.

Can we remove it?

---

Refs:

Motivation for babel-flow #5460 (comment) and #5460 (comment)

PR introduce babel-flow #5685

[thorn0]

found it will cost almost half time on some file by removing plugins.

Could you show some benchmark results?

Can we remove it?

I guess we can if we make Prettier switch to babel-flow automatically when the @flow pragma is found.

[fisker]

Could you show some benchmark results?

Will do.

I guess we can if we make Prettier switch to babel-flow automatically when the @flow pragma is found.

My intention is not support flow at all, but check @flow is a good idea, so we don't break anything.

[fisker]

Run against src/**/*

Test script

const fs = require('fs')
const oldParse = require('./src/language-js/parser-babel-master').parsers.babel.parse
const newParse = require('./src/language-js/parser-babel').parsers.babel.parse
const files = require('fast-glob').sync('src/**/*').map(file => fs.readFileSync(file, 'utf8'))
console.log('first run')

console.time('new')

for (const code of files) {

newParse(code)

}

console.timeEnd('new')

console.time('old')

for (const code of files) {

oldParse(code)

}

console.timeEnd('old')
console.log()

console.log('second run')
console.time('old')

for (const code of files) {

oldParse(code)

}

console.timeEnd('old')
console.time('new')

for (const code of files) {

newParse(code)

}

console.timeEnd('new')

first run
new: 517.260ms
old: 594.656ms

second run
old: 332.977ms
new: 186.684ms

---

Run against this 18M js file from here

Test script

const fs = require('fs')

const oldParse = require('./src/language-js/parser-babel-master').parsers.babel.parse

const newParse = require('./src/language-js/parser-babel').parsers.babel.parse
const code = fs.readFileSync('./AngryBots.js', 'utf8')
console.log('first run')

console.time('new')

newParse(code)

console.timeEnd('new')

console.time('old')

oldParse(code)

console.timeEnd('old')
console.log()

console.log('second run')
console.time('old')

oldParse(code)

console.timeEnd('old')
console.time('new')

newParse(code)

console.timeEnd('new')

Actual Babel options (jsx flow and some other plugins are not enabled)

[ { sourceType: 'module',
    allowAwaitOutsideFunction: true,
    allowImportExportEverywhere: true,
    allowReturnOutsideFunction: true,
    allowSuperOutsideMethod: true,
    allowUndeclaredExports: true,
    errorRecovery: true,
    createParenthesizedExpressions: true,
    plugins:
     [ 'asyncGenerators',
       'bigInt',
       'doExpressions',
       'exportDefaultFrom',
       'functionBind',
       'functionSent',
       'importMeta',
       'numericSeparator',
       'objectRestSpread',
       'optionalCatchBinding',
       'partialApplication',
       'throwExpressions',
       'v8intrinsic' ] } ]

Result

first run
new: 26619.009ms
old: 47839.521ms

second run
old: 46681.359ms
new: 27487.509ms

---

The prototype can be found here.

[fisker]

@thorn0 I've done removing flow (also simple tests for each babel plugin) from babel parser
fisker#378 , it indeed makes the parse faster on pure js projects, but this is a breaking change.

Before,we can parse flow syntax by default.
After, users need use @flow or switch to babel-flow parser, should I send the PR?

[thorn0]

Do people write Flow code in files with .js extension and without @flow?

[fisker]

I don't use flow , but some tests in tests/flow/ don't has @flow

[thorn0]

We can't proceed with this if people do that.

[fisker]

Shouldn't we encourage people use flow or babel-flow?

About this, what do you think?

[thorn0]

Prettier should "just work", even without config files. But if people write Flow code in .js files, they'll have to know to create a config file to specify parser.

[thorn0]

Copying my comment from #8508:

That's fine. The babel parser isn't safe for Flow anyway: https://prettier.io/blog/2019/01/20/1.16.0.html#flow
I agree that it's a mess. BTW, probably this means that if we proceed with #8204 (parse Flow syntax only if the pragma is present), it won't really be a breaking change as we'd break something that is already broken.

[fisker]

Fixed by #14314