Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An in-range update of prettier is breaking the build 🚨 #921

Closed
greenkeeper bot opened this issue Apr 19, 2017 · 4 comments
Closed

An in-range update of prettier is breaking the build 🚨 #921

greenkeeper bot opened this issue Apr 19, 2017 · 4 comments

Comments

@greenkeeper
Copy link
Contributor

greenkeeper bot commented Apr 19, 2017

Version 1.2.0 of prettier just got published.

Branch Build failing 🚨
Dependency prettier
Current Version 1.1.0
Type devDependency

This version is covered by your current version range and after updating it in your project the build failed.

As prettier is “only” a devDependency of this project it might not break production or downstream projects, but “only” your build or test tools – preventing new deploys or publishes.

I recommend you give this issue a high priority. I’m sure you can resolve this 💪


Status Details
  • continuous-integration/travis-ci/push The Travis CI build is in progress Details

  • Better Code Hub ✅ Better Code Hub approves this code Details

  • bitHound - Dependencies No failing dependencies. Details

  • bitHound - Code 310 failing files. Details

Commits

The new version differs by 26 commits .

  • bc53920 1.2.0
  • 3dc7562 Don't inline paren at right of arguments (#1345)
  • aafcf5f Break if () if conditional inside breaks (#1344)
  • 042e603 Fix arrow function parenthesis with comments in flow (#1339)
  • 938f0e3 Improve regex printing (#1341)
  • e2fbaaf Update link to @vjeux's React London presentation (#1330)
  • 706640d Optimize prettier --help for humans (#1340)
  • cb79d82 add printer branch for TSFirstTypeNode (#1332)
  • dc499ba Add information about Vim's other autocmd events (#1333)
  • 565106d Add parentheses for assignment as body of arrow (#1326)
  • 652e2f9 Add prettier_d to Related Projects (#1328)
  • 2e613cb Add jestbrains filewatcher docs (#1310)
  • 5e7503d Add typescript as a valid parser value (#1318)
  • 8d03423 Avoid breaking arguments for last arg expansion (#1305)
  • 5995af2 Bail when traversing === groups (#1294)

There are 26 commits in total. See the full diff.

Not sure how things should work exactly?

There is a collection of frequently asked questions and of course you may always ask my humans.


Your Greenkeeper Bot 🌴

@greenkeeper
Copy link
Contributor Author

greenkeeper bot commented Apr 19, 2017

Version 1.2.1 just got published.

Your tests are still failing with this version. Compare the changes 🚨

Commits

The new version differs by 2 commits .

See the full diff.

@greenkeeper
Copy link
Contributor Author

greenkeeper bot commented Apr 19, 2017

Version 1.2.2 just got published.

Your tests are still failing with this version. Compare the changes 🚨

Commits

The new version differs by 2 commits .

See the full diff.

@greenkeeper
Copy link
Contributor Author

greenkeeper bot commented May 2, 2017

Version 1.3.0 just got published.

Your tests are still failing with this version. Compare the changes 🚨

Release Notes 1.3.0

Facebook Adoption Update

The reason why I (@vjeux) embarked on this journey working on prettier has always been to get the entire Facebook codebase converted over. I would like to give an update on how it is going and what is the process to get there.

The first projects to adopt prettier were Jest, React and immutable-js. Those are small codebases in the order of hundreds of files that have their own infrastructure. There are 5 or less people working on them full time.

Then, Oculus and Nuclide converted their codebase over. The scale is bigger with a few thousands of files and tens of full time contributors but looks pretty similar to the first projects. The conversions went in one big codemod and that's it.

Now, the entire Facebook codebase is way bigger than this and it's not feasible to just convert everything in one go and to convince everyone that their entire codebase is going to be reformatted under their feet. So we need to find a more incremental approach.

Scaling adoption

Running prettier on a piece of code is a pretty expensive operation, it makes your pull request look bad because of a lot of unrelated changes and it causes merge conflicts for all the outstanding pull requests. So once a file has been formatted, you should do everything to make sure it remains formatted.

  • When pretty-printing a file, add @format to the first block comment like @flow.
  • Have a lint rule with autofix that checks if the file is correctly pretty printed when @format is present.
    • When running Nuclide, it's going to show as an inline warning and have a fix button.
    • When sending a pull request, it's going to show the lint failing with a [Yn] prompt that you can just press enter.
  • Update the default code templates to add @format to the header.
  • When you run code formatting via cmd-shift-c inside of Nuclide, automatically insert the @format header.
  • Disable all the stylistic rules like max-len when @format is in the header.
  • Have script to run prettier through an entire folder with everything configured as a one line operation.
  • Have a good guide to help people that want to convert their codebase over with instructions and best practices.
  • When pushing a new release of prettier, also run it through all the files with @format in order to avoid getting warnings afterwards.
  • Add tracking for the number of files with @format over time.

We finally got all those things wired up 1.5 weeks ago and the reception has been insane. Many people from various teams converted their codebase to prettier on their own. As of today, 15% of Facebook codebase has been converted over!

When I started working on prettier, I had a hunch that people were hungry for tools to solve formatting. But I had no idea that once the tooling was in place, people would rush to convert their codebase over! This is great confirmation that this project is useful to people and not just a gimmicky tool.

TypeScript Support Progress

@despairblue, @azz and @JamesHenry have been hard at work around getting TypeScript supported by prettier as it's the top requested feature. 2000 out of 11000 files in the TypeScript test suite are not yet properly printed. You can follow progress on #1480 and do not hesitate to help out!

Flow

Add trailing commas on flow generics (#1381)

The --trailing-comma=all option is supposed to add trailing commas everywhere possible, but as an oversight we forgot to do it for flow generics.

// Before
type Errors = Immutable.Map<
  Ahohohhohohohohohohohohohohooh,
  Fbt | Immutable.Map<ErrorIndex, Fbt>
>;

// After
type Errors = Immutable.Map<
Ahohohhohohohohohohohohohohooh,
Fbt | Immutable.Map<ErrorIndex, Fbt>,
>;

Inline nullable in flow generics (#1426)

The phase after printing things correctly is to tweak the output to make it closer to the way people write code in practice. Inlining optional flow types is a small thing that makes a difference.

// Before
type Cursor = Promise<
  ?{
    newCursor?: number,
    formatted: string,
  }
>;

// After
type Cursor = Promise<?{
newCursor?: number,
formatted: string,
}>;

Allow flow declarations to break on StringLiteralTypeAnnotations (#1437)

We can always find more places to add breaks when things don't fit 80 columns. This time it's around declaring a type as a constant string.

// Before
export type AdamPlacementValidationSingleErrorKey = 'SOME_FANCY_TARGETS.GLOBAL_TARGET';

// After
export type AdamPlacementValidationSingleErrorKey =
'SOME_FANCY_TARGETS.GLOBAL_TARGET';

Add space around = for flow generics default arguments (#1476)

Another example of small thing where we can improve the display of flow code. For function default arguments we put a space around = but didn't around flow generics.

// Before
class PolyDefault<T=string> {}

// After
class PolyDefault<T = string> {}

Don't break for unparenthesised single argument flow function (#1452)

I'm trying to figure out something to write here, but ... it just looks weird!

// Before
const selectorByPath:
  Path
 => SomethingSelector<
  SomethingUEditorContextType,
  SomethingUEditorContextType,
  SomethingBulkValue<string>
> = memoizeWithArgs(/* ... */)

// After
const selectorByPath: Path => SomethingSelector<
SomethingUEditorContextType,
SomethingUEditorContextType,
SomethingBulkValue<string>
> = memoizeWithArgs(/* ... */);

Fix optional flow parenthesis (#1357)

We were a bit too lenient around parenthesis for optional flow types. In one case in the entire Facebook codebase, it generated code with different semantics. As part of this fix, we hardened the list of types that can be written without parenthesis.

// Before
type X = ?(number, number) => number => void;

// After
type X = (?(number, number) => number) => void;

Skip trailing commas with FlowShorthandWithOneArg (#1364)

It is a parse error to add a trailing comma without parenthesis for arguments of arrow function types. We found one case in Facebook codebase when this happened, it's a very rare occurrence.

// Before
type IdeConnectionFactory =
  child_process$ChildProcess,
  => FlowIDEConnection = defaultIDEConnectionFactory;

// After
type IdeConnectionFactory =
child_process$ChildProcess,
=> FlowIDEConnection = defaultIDEConnectionFactory;

Reorder flow object props (#1451)

This one is an example where the way the AST is structured is not our favor. Instead of having a list of elements inside of a type, the AST is structured in a way where normal keys and array keys each have their own group. In order to restore the initial order, we're now reading from the original source :(

// Before
type Foo = {
  [key: string]: void,
  alpha: "hello",
  beta: 10
};

// After
type Foo = {
alpha: 'hello',
[key: string]: void,
beta: 10
}

Template Literal

Proper indentation for template literals (#1385)

A long standing issue with template literals and prettier is around the indentation of code inside of ${}. It used to be the indentation of the backtick but turned out to give poor results. Instead people tend to use the indent of the ${. We changed this behavior and it magically made GraphQL queries look pretty!

// Before
Relay.createContainer({
  nodes: ({ solution_type, time_frame }) => Relay.QL`
    fragment {
      __typename
      ${OptimalSolutionsSection.getFragment("node", {
    solution_type,
    time_frame
  })}
    }
  `
})
// After
Relay.createContainer({
  nodes: ({ solution_type, time_frame }) => Relay.QL`
    fragment {
      __typename
      ${OptimalSolutionsSection.getFragment("node", {
        solution_type,
        time_frame
      })}
    }
  `
})

Do not indent calls with a single template literal argument (#873)

Template literals are very hard to deal with for a pretty printer because the spaces inside are meaningful so you can't re-indent them. We didn't know what to do for a call with a single template literal so we didn't do anything, but we kept receiving reports of people saying that prettier indented it the wrong way, so we are now inlining them. Fingers crossed it is going to cover most use cases.

// Before
insertRule(
  `*, *:before, *:after {
    box-sizing: inherit;
  }`
);

// After
insertRule(</span>*, *:before, *:after {</span> <span class="pl-s"> box-sizing: inherit;</span> <span class="pl-s">}<span class="pl-pds">);

Fix windows line ending on template literals (#1439)

We manipulate line endings in a lot of places in prettier and took great care of handling both \n and \r\n except for template literals where we forgot. Now this is fixed!

// Before
const aLongString = `

Line 1

Line 2

Line 3

`;

// After
const aLongString = </span></span> <span class="pl-s">Line 1</span> <span class="pl-s">Line 2</span> <span class="pl-s">Line 3</span> <span class="pl-s"><span class="pl-pds">;

Inline template literals as arrow body (#1485)

We already inline template literals that are tagged (eg graphql`query`) but didn't for plain template literals. For the anecdote, it turns out the code was supposed to support it but it was using TemplateElement instead of TemplateLiteral :(

// Before
const inlineStore = preloadedState =>
  `
  <script>
    window.preloadedState = ${JSON.stringify(preloadedState).replace(/</g, '\\u003c')}
  </script>
`

// After
const inlineStore = preloadedState => </span></span> <span class="pl-s"> &lt;script&gt;</span> <span class="pl-s"> window.preloadedState = <span class="pl-s1"><span class="pl-pse">${</span><span class="pl-c1">JSON</span>.<span class="pl-en">stringify</span>(preloadedState).<span class="pl-c1">replace</span>(<span class="pl-sr"><span class="pl-pds">/</span>&lt;<span class="pl-pds">/</span>g</span>, <span class="pl-s"><span class="pl-pds">'</span><span class="pl-cce">\\</span>u003c<span class="pl-pds">'</span></span>)<span class="pl-pse">}</span></span></span> <span class="pl-s"> &lt;/script&gt;</span> <span class="pl-s"><span class="pl-pds">

Ternaries

Add parenthesis for unusual nested ternaries (#1386)

While working on printing nested ternaries, everyone focused on the ones with the shape of an if then else cond1 ? elem1_if : cond2 ? elem2_if : elem_else which is the most common. But, if you move move some ? and : around you can have another pattern. It looks almost the same but has a different meaning. In order to reduce confusion, we're adding parenthesis around the uncommon form.

// Before
cond1 ? cond2 ? elem2_if : elem2_else : elem1_else

// After
cond1 ? (cond2 ? elem2_if : elem2_else) : elem1_else

Only add parenthesis on ternaries inside of arrow functions if doesn't break (#1450)

There's an eslint rule no-confusing-arrows which suggests adding parenthesis for ternaries in arrow functions without brackets.

var x = a => 1 ? 2 : 3;
var x = a <= 1 ? 2 : 3;

It makes sense when code is in one line, but when it is split into multiple lines, the parenthesis are unnecessary given the indentation, so we now only put them when they serve their disambiguation purpose.

// Before
var x = a => (1 ? 2 : 3);
var x = a =>
  (1
    ? 2
    : 3);

// After
var x = a => (1 ? 2 : 3);
var x = a =>
1
? 2
: 3;

General JavaScript Improvements

Inline function declaration with single arg as object (#1173)

This one was often requested for React Stateless Functional Components (SFC). If you make use of a lot of them, it's likely going to be a big change for you.

// Before
const X = (
  props: {
    a: boolean,
  },
) => <div />;

// After
const X = (props: {
a: boolean,
}) => <div />;

Break inline object first in function arguments (#1453)

One thing we discovered early on is that people usually break the arguments of the function before breaking the return type. Unfortunately, the code responsible to inline single destructuring argument broke that assumption and it introduced bad looking code like this example. The good news is that it enables us to turn on inlining for single arguments that are typed with an object.

// Before
class X {
  async onDidInsertSuggestion({editor, triggerPosition, suggestion}): Promise<
    void
  > {
  }
}

// After
class X {
async onDidInsertSuggestion({
editor,
triggerPosition,
suggestion
}): Promise<void> {
}
}

Don't break on empty arrays and objects (#1440)

This one has been a long standing issue and is an easy fix, but was an invaluable tool: whenever someone reported that [] or {} would break, we were able to fix the example by fixing something else. So it was a great way to surface edge cases. Fortunately, this vein has now ran out and all the recent examples just look bad with no other reason than the fact that they are breaking. So it's time to finally do it!

// Before
const a = someVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeLong.Expression || [
];

// After
const a = someVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeLong.Expression || [];

Do not break on [0] (#1441)

We have a lot of issues where code breaks in array access when it doesn't look good. We don't yet have a good generic solution for it, but we can add a specific fix a common situation: [0].

// Before
queryThenMutateDOM(() => {
  title = SomeThing.call(root, "someLongStringThatPushesThisTextReall")[
    0
  ];
});

// After
queryThenMutateDOM(() => {
title = SomeThing.call(
root,
"someLongStringThatPushesThisTextReall",
)[0];
});

Indent do while condition (#1373)

We were not using the correct indentation logic for do-while condition but someone noticed, now we do!

// Before
do {}
while (someVeryLongStringA && someVeryLongStringB && someVeryLongStringC && someVeryLongStringD);

// After
do {}
while (
someVeryLongStringA &&
someVeryLongStringB &&
someVeryLongStringC &&
someVeryLongStringD
);

Preserve inline comment as last argument (#1390)

We forgot to add one case in the comment detection code when they appear last for JSX attributes and function arguments which made them go after the closing. In the case of JSX, it generated code that had a different meaning. Fortunately, since we don't usually commit commented out code it didn't affect production code, but it is not a good experience while coding.

// Before
const x = (
  <div
    attr1={1}>
//   attr3={3}
    {children}
  </div>
);

// After
const x = (
<div
attr1={1}
// attr3={3}
>
{children}
</div>
);

Break class expression returned by arrow call (#1464)

In 1.0, we made class be inline inside of arrow functions. It turns out that it doesn't work great when the class is non trivial, so we are reverting this change. We're trying really hard to avoid making trashy decisions like this where the style changes back and forth, but we allow ourselves to do it sometimes to fix mistakes!

// Before
export default (ViewComponent: Function, ContainerComponent: Function) => class
  extends React.Component {
  static propTypes = {};
};

// After
export default (ViewComponent: Function, ContainerComponent: Function) =>
class extends React.Component {
static propTypes = {};
};

Fix empty line in block with EmptyStatement (#1375)

This one was found by fuzzing. You're unlikely going to hit this in real code but it's good to know it is fixed!

// Input
if (a) {
  b;

;
}

// Before
if (a) {
b;

}

// After
if (a) {
b;

}

Commits

The new version differs by 48 commits0.

  • a81d5c1 1.3.0
  • 0785726 Inline template literals as arrow body (#1485)
  • f59aeef Break inline object first in function arguments (#1453) (#1173)
  • 8f9bb3a Break inline object first in function arguments (#1453)
  • 54b8cac Reorder flow object props (#1451)
  • c99a877 Do not break on [0] (#1441)
  • acfb14f Don't break on empty arrays and objects (#1440)
  • bafd724 Don't break for unparenthesised single argument flow function (#1452)
  • a335c26 Add space around = for flow generics default arguments (#1476)
  • 4b7d265 Fix windows line ending on template literals (#1439)
  • e392093 Only add parenthesis on ternaries inside of arrow functions if doesn't break (#1450)
  • 93cad97 Preserve inline comment as last argument (#1390)
  • 314e963 Add parenthesis for unusual nested ternaries (#1386)
  • 13f05fb Proper indentation for template literals (#1385)
  • c521406 [RFC] Do not indent calls with a single template literal argument (#873)

There are 48 commits in total.

See the full diff

@greenkeeper
Copy link
Contributor Author

greenkeeper bot commented May 3, 2017

Version 1.3.1 just got published.

Your tests are still failing with this version. Compare the changes 🚨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant