Break long member and method chain #15171

seiyab · 2023-07-29T11:46:09Z

Description

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.

seiyab · 2023-07-29T14:00:48Z

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

+      path.call((object) => rec(object), "object");
+      printedNodes.push({


I reverted this change because objectDoc doesn't look necessary here.

seiyab · 2023-07-29T14:01:05Z

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

+                path,
+                options,
+                print,
+                printedNodes.at(-1)?.printed,


Outdated. This should be resolved.

seiyab · 2023-07-29T12:01:49Z

tests/format/js/arrow-call/__snapshots__/jsfmt.spec.js.snap

-expect(() =>
-  asyncRequest({ type: "foo", url: "/test-endpoint" })
-).not.toThrowError();
+expect(() => asyncRequest({ type: "foo", url: "/test-endpoint" })).not
+  .toThrowError();


It looks to get worse to read.
Currently, I consider this to be another issue.

seiyab · 2023-07-29T12:35:47Z

src/language-js/print/member.js

+
+/**
+ * @param {AstPath} path
+ * @param {*} objectDoc


I want to put "Doc" type here but it requires too redundant check like:

- objectDoc.label?.memberChain + typeof objectDoc === "object" && "label" in objectDoc && typeof objectDoc.label === "object" && "memberChain" in objectDoc.label && objectDoc.label?.memberChain

Adding @property {*} [label] in Doc (it should be right), it get a bit better:

- objectDoc.label?.memberChain + typeof objectDoc === "object" && "label" in objectDoc && objectDoc.label?.memberChain

But I think this might still make developers to avoid typing.
Defining @typedef {Partial<DocObject> & (string | DocObject | DocArray)} LooseDoc, or just defining Doc as it make just objectDoc.label?.memberChain works.
I'd like to hear member's opinion

I mean that I will open a discussion or an issue if it is worth to discuss, or open a PR if it looks better than the current one.

seiyab · 2023-07-30T08:15:27Z

Maybe I accidentally hit another independent issue about idempotency...

-   .Even// comment<LF>
+   .Even // comment<LF>

seiyab · 2023-08-15T12:26:49Z

I consider it ready for review.
(Just for a reminder. I'm patiently waiting for review.)

fisker · 2023-08-25T16:36:06Z

Sorry for the long delay, I'll take a look ASAP.

fisker · 2023-08-28T02:22:14Z

tests/format/js/method-chain/__snapshots__/jsfmt.spec.js.snap

+  .Even // comment
+  .if.Prettier.does.not.format.all.code["100%"].the.way["you’d"].like["it’s"]
+    .worth.the.sacrifice.given.the.unique.benefits.of.Prettier["don’t"].you
+    .think?.();


Can you find some real world case? These tests doesn't make sense to me at all. Nobody write code like this.

I see. I'll consider a more practical test case. This is just a test case that is so long chain and not likely to have a copyright / license issue.

fisker · 2023-08-28T02:23:08Z

tests/format/js/method-chain/__snapshots__/jsfmt.spec.js.snap

+    "YAML",
+  )
+  .it.removes.all.original.styling.and.ensures.that.all.outputted.code.conforms
+    .to.a.consistent.style(see, this, blogpost);


This indention seems unnessary.

I'll fix it.

seiyab · 2023-09-04T13:51:38Z

Progress
I'm still working on this.
It'll take for a while because it looks harder than I had thought and I get busier a bit now.

seiyab · 2023-09-18T08:29:13Z

Finally it looks to "work".
But I'm not sure whether the introduced complexity is reasonable to fix the bug.
I intend to look for simpler way. This might be more costly than leaving the bug existing.

seiyab · 2023-09-21T13:35:55Z

I give up to make it easier to read.
I consider it's ready for review.

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

seiyab added 5 commits July 29, 2023 14:33

add as-is test

7770728

fix prettier#2548

9920e6a

update snapshots

7c56d8e

add test

6065709

refactor

946c90e

seiyab commented Jul 29, 2023

View reviewed changes

delete a file I mistakenly commited

6703814

seiyab commented Jul 29, 2023

View reviewed changes

seiyab added 4 commits July 29, 2023 21:55

write changelog

69ee580

don't pass objectDoc in method chain

104e407

revert member-chain.js

c6c9d6b

change coding style into probably preferred one in this project

727efa7

seiyab changed the title ~~2548 long long member and method chain~~ Break long member and method chain Jul 29, 2023

seiyab marked this pull request as ready for review July 29, 2023 14:08

add test

10ef048

avoid (probably) unrelated issue

2073238

seiyab mentioned this pull request Jul 30, 2023

Strange formatting if a member expression is a callee and the chain is longer than the line length #15026

Open

seiyab requested a review from fisker August 25, 2023 13:43

fisker reviewed Aug 28, 2023

View reviewed changes

seiyab added 2 commits September 2, 2023 11:29

avoid indent for member in tail groups

fcd3d66

change test cases

ddcf1ca

seiyab added 2 commits September 14, 2023 22:31

fix some bugs around linebreak / indent

f31596c

update snapshots

b30098e

seiyab added 3 commits September 16, 2023 17:55

fix a bug relates to inlining

21fe696

remove an unnecessary line

6741ed8

resolve idempotency problem

4c5265c

seiyab requested a review from fisker September 21, 2023 13:36

Merge branch 'main' into 2548-long-long-member-and-method-chain

3bb92d4

seiyab mentioned this pull request Oct 16, 2023

Support (type) satisfies never; #15514

Merged

4 tasks

fisker reviewed Oct 17, 2023

View reviewed changes

src/language-js/print/member-chain.js Outdated Show resolved Hide resolved

seiyab added 2 commits October 18, 2023 22:31

remove useless line

78ac561

Merge branch 'main' into 2548-long-long-member-and-method-chain

99d389d

seiyab requested a review from fisker October 18, 2023 13:57

seiyab mentioned this pull request Oct 19, 2023

Stabilize member chain with empty line #15522

Merged

4 tasks

Merge branch 'main' into 2548-long-long-member-and-method-chain

20441e0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Break long member and method chain #15171

Break long member and method chain #15171

seiyab commented Jul 29, 2023 •

edited

This comment was marked as outdated.

seiyab Jul 29, 2023

This comment was marked as resolved.

seiyab Jul 29, 2023 •

edited

seiyab Jul 29, 2023 •

edited

This comment was marked as outdated.

seiyab Jul 29, 2023 •

edited

seiyab Aug 5, 2023

seiyab commented Jul 30, 2023

seiyab commented Aug 15, 2023

fisker commented Aug 25, 2023

fisker Aug 28, 2023

seiyab Aug 29, 2023

fisker Aug 28, 2023

seiyab Aug 29, 2023

seiyab commented Sep 4, 2023

seiyab commented Sep 18, 2023

seiyab commented Sep 21, 2023

		path.call((object) => rec(object), "object");
		printedNodes.push({

Break long member and method chain #15171

Are you sure you want to change the base?

Break long member and method chain #15171

Conversation

seiyab commented Jul 29, 2023 • edited

Description

Checklist

This comment was marked as outdated.

seiyab Jul 29, 2023

Choose a reason for hiding this comment

This comment was marked as resolved.

seiyab Jul 29, 2023 • edited

Choose a reason for hiding this comment

seiyab Jul 29, 2023 • edited

Choose a reason for hiding this comment

This comment was marked as outdated.

seiyab Jul 29, 2023 • edited

Choose a reason for hiding this comment

seiyab Aug 5, 2023

Choose a reason for hiding this comment

seiyab commented Jul 30, 2023

seiyab commented Aug 15, 2023

fisker commented Aug 25, 2023

fisker Aug 28, 2023

Choose a reason for hiding this comment

seiyab Aug 29, 2023

Choose a reason for hiding this comment

fisker Aug 28, 2023

Choose a reason for hiding this comment

seiyab Aug 29, 2023

Choose a reason for hiding this comment

seiyab commented Sep 4, 2023

seiyab commented Sep 18, 2023

seiyab commented Sep 21, 2023

seiyab commented Jul 29, 2023 •

edited

seiyab Jul 29, 2023 •

edited

seiyab Jul 29, 2023 •

edited

seiyab Jul 29, 2023 •

edited