Use prettier by danez · Pull Request #390 · babel/babylon

danez · 2017-03-01T11:18:43Z

The changes are very minimal, only in state.js prettier created a super long line, but we could also optimize this by hand and instead initialize every property individual.

Thoughts?

add to pre-commit hook + travis
adjust eslint

codecov · 2017-03-01T11:18:45Z

Codecov Report

Merging #390 into master will increase coverage by 0.07%.
The diff coverage is 98.41%.

@@            Coverage Diff             @@
##           master     #390      +/-   ##
==========================================
+ Coverage   98.14%   98.21%   +0.07%     
==========================================
  Files          22       22              
  Lines        3663     3988     +325     
  Branches     1024     1219     +195     
==========================================
+ Hits         3595     3917     +322     
- Misses         25       28       +3     
  Partials       43       43

Flag	Coverage Δ
#babel	`80.94% <72.15%> (+1.28%)`	⬆️
#babylon	`96.99% <97.81%> (+0.15%)`	⬆️

Impacted Files	Coverage Δ
src/plugins/jsx/xhtml.js	`100% <ø> (ø)`	⬆️
src/plugins/flow.js	`98.47% <ø> (+0.07%)`	⬆️
src/parser/base.js	`100% <ø> (ø)`	⬆️
src/options.js	`100% <ø> (ø)`	⬆️
src/util/location.js	`100% <ø> (ø)`	⬆️
src/parser/location.js	`100% <100%> (ø)`	⬆️
src/tokenizer/types.js	`100% <100%> (ø)`	⬆️
src/parser/util.js	`90.62% <100%> (+0.62%)`	⬆️
src/parser/node.js	`96.66% <100%> (ø)`	⬆️
src/parser/statement.js	`99.2% <100%> (+0.08%)`	⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc87d99...b439cbd. Read the comment docs.

existentialism · 2017-03-01T15:55:26Z

We'll probably want a eslint-config-babel that contains no styling rules (or also extend https://github.com/prettier/eslint-config-prettier)?

vjeux · 2017-03-03T16:04:53Z

This is super exciting! Do not hesitate to open up issues on the prettier repo if you see anything that isn't printed in a way that you would have written code. I want to know about it :)

vjeux · 2017-03-03T16:58:34Z

src/tokenizer/state.js

-      this.inType =
-      this.noAnonFunctionType =
-        false;
+    this.inMethod = this.inFunction = this.inGenerator = this.inAsync = this.inPropertyName = this.inType = this.noAnonFunctionType = false;


I had no idea people would chain this kind of calls! We could support it in prettier but that would add quite a bit of complexity. Since you are already writing them each on their own line, I think that it would be better to just rewrite is as

this.inMethod = false; this.inFunction = false; // ...

It's probably specific to the parser/acorn, not common, we could probably just ignore for certain sections that aren't going to change much

@vjeux Yes I mentioned that in the description above too. It is probably more readable if we rewrite it.

feel free to add // @prettier-ignore just before if you want to keep formatting for some special cases.

One case I know of is Babel actually outputs code like this in the case of exports at the moment. See https://babeljs.io/repl/#?babili=false&evaluate=false&lineWrap=false&presets=es2015&targets=&browsers=&builtIns=false&code=import%20thing%20from%20'bar'%3B%0A%0Aexport%20var%20a%2C%20b%2C%20c%2C%20d%2C%20e%2C%20f%2C%20g%2C%20h%2C%20i%2C%20j%2C%20k%2C%20l%2C%20m%2C%20n%20%2Co%2C%20p%2C%20q%2C%20r%2C%20s%2C%20t%2C%20u%2C%20v%2C%20x%2C%20y%2C%20z%3B&experimental=true&loose=false&spec=false&playground=true&stage=0

includes

exports.z = exports.y = exports.x = exports.v = exports.u = exports.t = exports.s = exports.r = exports.q = exports.p = exports.o = exports.n = exports.m = exports.l = exports.k = exports.j = exports.i = exports.h = exports.g = exports.f = exports.e = exports.d = exports.c = exports.b = exports.a = undefined;

That's just output though so it's fine?

vjeux · 2017-03-03T16:59:41Z

src/util/identifier.js

 // offset to the next range, and then a size of the range. They were
 // generated by `bin/generate-identifier-regex.js`.
 // eslint-disable-next-line comma-spacing
-const astralIdentifierStartCodes = [0,11,2,25,2,18,2,1,2,14,3,13,35,122,70,52,268,28,4,48,48,31,17,26,6,37,11,29,3,35,5,7,2,4,43,157,19,35,5,35,5,39,9,51,157,310,10,21,11,7,153,5,3,0,2,43,2,1,4,0,3,22,11,22,10,30,66,18,2,1,11,21,11,25,71,55,7,1,65,0,16,3,2,2,2,26,45,28,4,28,36,7,2,27,28,53,11,21,11,18,14,17,111,72,56,50,14,50,785,52,76,44,33,24,27,35,42,34,4,0,13,47,15,3,22,0,2,0,36,17,2,24,85,6,2,0,2,3,2,14,2,9,8,46,39,7,3,1,3,21,2,6,2,1,2,4,4,0,19,0,13,4,159,52,19,3,54,47,21,1,2,0,185,46,42,3,37,47,21,0,60,42,86,25,391,63,32,0,449,56,264,8,2,36,18,0,50,29,881,921,103,110,18,195,2749,1070,4050,582,8634,568,8,30,114,29,19,47,17,3,32,20,6,18,881,68,12,0,67,12,65,0,32,6124,20,754,9486,1,3071,106,6,12,4,8,8,9,5991,84,2,70,2,1,3,0,3,1,3,3,2,11,2,0,2,6,2,64,2,3,3,7,2,6,2,27,2,3,2,4,2,0,4,6,2,339,3,24,2,24,2,30,2,24,2,30,2,24,2,30,2,24,2,30,2,24,2,7,4149,196,60,67,1213,3,2,26,2,1,2,0,3,0,2,9,2,3,2,0,2,0,7,0,5,0,2,0,2,0,2,2,2,1,2,0,3,0,2,0,2,0,2,0,2,0,2,1,2,0,3,3,2,6,2,3,2,3,2,0,2,9,2,16,6,2,2,4,2,16,4421,42710,42,4148,12,221,3,5761,10591,541];


fyi, you can use // @prettier-ignore in order to print it as is and not have every argument expanded in a single line.

vjeux · 2017-03-03T17:02:30Z

src/tokenizer/index.js

-    if (isIdentifierStart(code) || code === 92 /* '\' */) {
-      return this.readWord();
+    if (isIdentifierStart(code) || code === 92) {
+      /* '\' */ return this.readWord();


This is not intended, I just created an issue for it: prettier/prettier#867

vjeux · 2017-03-03T17:03:51Z

src/plugins/jsx/index.js


  if (!openingElement.selfClosing) {
-    contents: for (;;) {
+    contents:


This will be fixed in the next release: prettier/prettier#860

hzoo · 2017-03-03T17:12:48Z

Can we just target 7.0 for this? We can cherry pick other prs if necessary

vjeux · 2017-03-03T17:12:50Z

src/parser/statement.js

    case tt._import:
-      if (this.hasPlugin("dynamicImport") && this.lookahead().type === tt.parenL) break;
+      if (
+        this.hasPlugin("dynamicImport") && this.lookahead().type === tt.parenL


I opened prettier/prettier#868 as it looks a bit weird

loganfsmyth · 2017-03-03T18:57:39Z

src/plugins/jsx/fromCodePoint.js

+        // Astral code point; split in surrogate halves
        // https://mathiasbynens.be/notes/javascript-encoding#surrogate-formulae
        codePoint -= 0x10000;
-        highSurrogate = (codePoint >> 10) + 0xD800;


It's always so interesting how little things trigger knee-jerk reactions. I always prefer capitalized hex haha.

Yeah, this is pretty impressive!

For this one, we looked at github search results and it seems like the majority of js code uses lowercase. Also, Chrome devtools forces lowercase.

hzoo · 2017-03-21T21:10:20Z

We moved the previous 7.0 branch to be master now, and master is now 6.x

danez · 2017-04-21T14:24:41Z

I just rerun prettier 1.2.2 on the source and all the mentioned things are fixed now. :)

danez · 2017-06-09T21:43:52Z

Okay rebased and change everything to work. Ready for review.

hzoo · 2017-06-09T22:20:39Z

bin/generate-identifier-regex.js

 const startData = generate(start);
 const contData = generate(cont);

-console.log("let nonASCIIidentifierStartChars = \"" + startData.nonASCII + "\";");


We can just use template strings here, don't have to change in this pr

hzoo · 2017-06-09T22:21:53Z

src/parser/expression.js

-
-        this.finishNode(node, (op === tt.logicalOR || op === tt.logicalAND) ? "LogicalExpression" : "BinaryExpression");
-        return this.parseExprOp(node, leftStartPos, leftStartLoc, minPrec, noIn);
+        node.right = this.parseExprOp(


lol this whole block

hzoo · 2017-06-09T22:23:27Z

Just going to be annoying for @Andy-MS ts pr

bakkot · 2017-06-09T23:20:42Z

@hzoo after this lands he can just run prettier on his pr, presumably!

hzoo · 2017-06-10T00:10:48Z

That's the idea! I guess you would run beforehand and then rebase 7.0 and it would be ok?

hzoo · 2017-06-28T16:12:02Z

Moving to #600

danez added Tag: Internal PR: WIP labels Mar 1, 2017

danez changed the base branch from 7.0 to master March 1, 2017 11:19

vjeux reviewed Mar 3, 2017

View reviewed changes

loganfsmyth reviewed Mar 3, 2017

View reviewed changes

existentialism mentioned this pull request Mar 3, 2017

Use prettier babel/babel#5412

Merged

2 tasks

SimenB mentioned this pull request Mar 3, 2017

Use/recommend prettier schibsted/eslint-config-schibsted#70

Closed

existentialism mentioned this pull request Mar 9, 2017

Drop style rules babel/eslint-config-babel#22

Merged

hzoo added the Priority: High label Mar 30, 2017

Use prettier

b439cbd

danez force-pushed the prettier branch from b04f1f3 to b439cbd Compare June 9, 2017 21:42

hzoo reviewed Jun 9, 2017

View reviewed changes

existentialism mentioned this pull request Jun 28, 2017

Use prettier #600

Merged

hzoo closed this Jun 28, 2017

hzoo deleted the prettier branch June 30, 2017 11:06

Uh oh!

Conversation

danez commented Mar 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

existentialism commented Mar 1, 2017

Uh oh!

vjeux commented Mar 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo Mar 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danez Mar 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo commented Mar 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo commented Mar 21, 2017

Uh oh!

danez commented Apr 21, 2017

Uh oh!

danez commented Jun 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo commented Jun 9, 2017

Uh oh!

bakkot commented Jun 9, 2017

Uh oh!

hzoo commented Jun 10, 2017

Uh oh!

hzoo commented Jun 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

danez commented Mar 1, 2017 •

edited

Loading

codecov bot commented Mar 1, 2017 •

edited

Loading

hzoo Mar 3, 2017 •

edited

Loading

danez Mar 3, 2017 •

edited

Loading