Stabilize member chain with empty line

[seiyab]

Description

Fix #15435

Fortunately, this doesn't conflict with #15171.
I actually implemented this on the top of #15171 (99d389d) and rebased it into master without seeing any problems.

Checklist

• I’ve added tests to confirm my change works.
• (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
• (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
• I’ve read the contributing guidelines.

✨Try the playground for this PR✨

[seiyab]

I'm not sure whether it should be something like group.some(willBreak) as

prettier/src/language-js/print/member-chain.js

Line 398 in 51e48e5

printedGroups.slice(0, -1).some(willBreak) ||

.
Probably it will affect code like following:

this.bar(a).baz(
  <ul>
    <li>a</li>
  </ul>,
);

[fisker]

Can you add test for it?

[fisker]

Should we name the element as printedNodes?

[seiyab]

Ah, I was wrong. Trying to add test for it, I found it isn't affected by groups.some((g) => g.at(-1).hasTrailingEmptyLine) vs willBreak(oneLine) (group.some(willBreak) that I suggested isn't correct).
Changing groups.some((g) => g.at(-1).hasTrailingEmptyLine) into willBreak(oneLine), some existing test are affected.

Example

stable and this PR

function f() {
  return this._getWorker(workerOptions)({
    filePath,
    hasteImplModulePath: this._options.hasteImplModulePath,
  }).then((metadata) => {
    // \`1\` for truthy values instead of \`true\` to save cache space.
    fileMetadata[H.VISITED] = 1;
    const metadataId = metadata.id;
    const metadataModule = metadata.module;
    if (metadataId && metadataModule) {
      fileMetadata[H.ID] = metadataId;
      setModule(metadataId, metadataModule);
    }
    fileMetadata[H.DEPENDENCIES] = metadata.dependencies || [];
  });
}

willBreak(oneLine)

function f() {
  return this._getWorker(workerOptions)({
    filePath,
    hasteImplModulePath: this._options.hasteImplModulePath,
  })
    .then((metadata) => {
      // \`1\` for truthy values instead of \`true\` to save cache space.
      fileMetadata[H.VISITED] = 1;
      const metadataId = metadata.id;
      const metadataModule = metadata.module;
      if (metadataId && metadataModule) {
        fileMetadata[H.ID] = metadataId;
        setModule(metadataId, metadataModule);
      }
      fileMetadata[H.DEPENDENCIES] = metadata.dependencies || [];
    });
}

I'd like to hear your opinion about

• whether we should find and add test case specifically focusing on it.
  • Just for information: Zod looks good library to search for practical method chains.
• which is better groups.some((g) => g.at(-1).hasTrailingEmptyLine) vs willBreak(oneLine)
  • IMO, groups.some((g) => g.at(-1).hasTrailingEmptyLine) looks slightly better because:
    • it looks slightly more readable to me for existing test cases
    • it doesn't change formatting which doesn't relate to Unstable formatting for chained calls #15435

For naming, another variable is already named as printedNodes. I'm sorry but probably my mistake (group.some(willBreak), which is not correct) confused you.

[seiyab]

@fisker Could you respond this, please?

[fisker]

Sorry for been busy, will take a look.

[seiyab]

I just worried you might have missed notification. There's no hurry. Thank you.

[thorn0]

It's best to avoid adding more uses of willBreak. Eventually, the existing use should be somehow removed from the member chain printing code too because it causes #2803.

[seiyab]

I found following inconsistency around here and this PR doesn't resolve it. Anyway, this PR resolves idempotency issue #15435. I don't matter closing this PR if member consider it needs discussion.

Prettier 3.0.3
Playground link

--parser typescript

Input:

new Set()
  
  .add(5)
  .add("some text");

new Set()  
  .add(5)

  .add("some text");

Output:

new Set().add(5).add("some text");

new Set()
  .add(5)

  .add("some text");

[fisker]

LGTM from the result.

[thorn0]

These changes don't look good prettier/prettier-regression-testing#342 (comment)

[seiyab]

@thorn0
It's because this branch wasn't up-to-date. The difference comes from #15547.
I added test and merge latest main.

[fisker]

Maybe the empty line only makes sense when the chain is long enough.