Repository: jlongster/prettier (js)

Issue:
Member Chain comments are eaten away
```js
function f() {
  return observableFromSubscribeFunction()
    // Debounce manually rather than using editor.onDidStopChanging so that the debounce time is
    // configurable.
    .debounceTime(debounceInterval);
}
```

turns into

```js
function f() {
  return observableFromSubscribeFunction()// Debounce manually rather than using editor.onDidStopChanging so that the debounce time is
  // configurable.
  .debounceTime(debounceInterval);
}
```

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22function%20f()%20%7B%5Cn%20%20return%20observableFromSubscribeFunction()%5Cn%20%20%20%20%2F%2F%20Debounce%20manually%20rather%20than%20using%20editor.onDidStopChanging%20so%20that%20the%20debounce%20time%20is%5Cn%20%20%20%20%2F%2F%20configurable.%5Cn%20%20%20%20.debounceTime(debounceInterval)%3B%5Cn%7D%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D

and then

```js
function f() {
  return observableFromSubscribeFunction()// configurable.
  .debounceTime(debounceInterval);
}
```

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22function%20f()%20%7B%5Cn%20%20return%20observableFromSubscribeFunction()%2F%2F%20Debounce%20manually%20rather%20than%20using%20editor.onDidStopChanging%20so%20that%20the%20debounce%20time%20is%5Cn%20%20%2F%2F%20configurable.%5Cn%20%20.debounceTime(debounceInterval)%3B%5Cn%7D%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D

and finally

```js
function f() {
  return observableFromSubscribeFunction().debounceTime(debounceInterval);
}
```

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22function%20f()%20%7B%5Cn%20%20return%20observableFromSubscribeFunction()%2F%2F%20configurable.%5Cn%20%20.debounceTime(debounceInterval)%3B%5Cn%7D%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D

Pull request:
Member chain comments
When there is a comment right before the first `.`, we want to force break such that the comment is printed on its own line.

Note: this needs to be based on-top of #667 as it always indent the first `.`

Fixes #613

Patch:
diff --git a/src/printer.js b/src/printer.js
index 32301d62a..b99391c46 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -2253,12 +2253,12 @@ function printMemberChain(path, options, print) {
   }
 
   const printedGroups = groups.map(printGroup);
-
   const oneLine = concat(printedGroups);
+  const hasComment = groups.length >= 2 && groups[1][0].node.comments;
 
   // If we only have a single `.`, we shouldn't do anything fancy and just
   // render everything concatenated together.
-  if (groups.length <= 2) {
+  if (groups.length <= 2 && !hasComment) {
     return group(oneLine);
   }
 
@@ -2268,6 +2268,11 @@ function printMemberChain(path, options, print) {
     printIndentedGroup(groups.slice(shouldMerge ? 2 : 1), hardline)
   ]);
 
+  // If there's a comment, we don't want to print in one line.
+  if (hasComment) {
+    return group(expanded);
+  }
+
   // If any group but the last one has a hard line, we want to force expand
   // it. If the last group is a function it's okay to inline if it fits.
   if (printedGroups.slice(0, -1).some(willBreak)) {


Test patch:
diff --git a/tests/comments/__snapshots__/jsfmt.spec.js.snap b/tests/comments/__snapshots__/jsfmt.spec.js.snap
index 1b6ba4899..246208c14 100644
--- a/tests/comments/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/comments/__snapshots__/jsfmt.spec.js.snap
@@ -364,14 +364,12 @@ export type BuckWebSocketMessage =
     };
 
 // Missing one level of indentation because of the comment
-const rootEpic = (actions, store) => combineEpics(...epics)(
-  actions,
-  store
-)// Log errors and continue.
-.catch((err, stream) => {
-  getLogger().error(err);
-  return stream;
-});
+const rootEpic = (actions, store) => combineEpics(...epics)(actions, store)
+  // Log errors and continue.
+  .catch((err, stream) => {
+    getLogger().error(err);
+    return stream;
+  });
 
 // Two extra levels of indentation because of the comment
 export type AsyncExecuteOptions =
@@ -593,14 +591,12 @@ export type BuckWebSocketMessage =
     };
 
 // Missing one level of indentation because of the comment
-const rootEpic = (actions, store) => combineEpics(...epics)(
-  actions,
-  store
-)// Log errors and continue.
-.catch((err, stream) => {
-  getLogger().error(err);
-  return stream;
-});
+const rootEpic = (actions, store) => combineEpics(...epics)(actions, store)
+  // Log errors and continue.
+  .catch((err, stream) => {
+    getLogger().error(err);
+    return stream;
+  });
 
 // Two extra levels of indentation because of the comment
 export type AsyncExecuteOptions =
diff --git a/tests/method-chain/__snapshots__/jsfmt.spec.js.snap b/tests/method-chain/__snapshots__/jsfmt.spec.js.snap
index daa999035..72838e2d1 100644
--- a/tests/method-chain/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/method-chain/__snapshots__/jsfmt.spec.js.snap
@@ -115,6 +115,23 @@ it(\"should group messages with same created time\", () => {
 "
 `;
 
+exports[`test comment.js 1`] = `
+"function f() {
+  return observableFromSubscribeFunction()
+    // Debounce manually rather than using editor.onDidStopChanging so that the debounce time is
+    // configurable.
+    .debounceTime(debounceInterval);
+}
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+function f() {
+  return observableFromSubscribeFunction()
+    // Debounce manually rather than using editor.onDidStopChanging so that the debounce time is
+    // configurable.
+    .debounceTime(debounceInterval);
+}
+"
+`;
+
 exports[`test first_long.js 1`] = `
 "export default function theFunction(action$, store) {
   return action$.ofType(THE_ACTION).switchMap(action => Observable
diff --git a/tests/method-chain/comment.js b/tests/method-chain/comment.js
new file mode 100644
index 000000000..7cc47760d
--- /dev/null
+++ b/tests/method-chain/comment.js
@@ -0,0 +1,6 @@
+function f() {
+  return observableFromSubscribeFunction()
+    // Debounce manually rather than using editor.onDidStopChanging so that the debounce time is
+    // configurable.
+    .debounceTime(debounceInterval);
+}