Functional composition experiments by thorn0 · Pull Request #8300 · prettier/prettier

thorn0 · 2020-05-13T13:44:09Z

Playing with different ideas to (partially) address #6921.

✨Try the playground for this PR✨
Or test it locally: npm install thorn0/prettier#functional-composition-experiments

The proposed solution tries to distinguish between calls to functions like addEventListener (Node.js callback style) and to functional-composition functions like createSelector from Reselect.
- Calls to "functions with callbacks" are formatted the same way they're formatted now: the last argument (function expression) is "hugged", i.e. isn't placed on a new line.
- For "functional-composition calls", every argument is placed on its own line.
- The detection is based on the presupposition that
  1. functional-composition calls aren't used as expression statements, and that
  2. the callee of such a call is a simple identifier.
- The second condition was added only because of Lodash functions like _.map to keep their last arguments hugged.
If the last argument is a concise arrow function it won't be hugged (arrow functions "glue" long lines at badly split places #4303).
Examples from Revisit function literal heuristic introduced in Prettier 1.19 #6921 added to the tests.

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/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

fisker · 2020-05-29T09:22:58Z

Run this on our code base, result https://github.com/fisker/prettier/pull/429/files

fisker · 2020-07-14T14:56:33Z

Accept this?

alexander-akait · 2020-07-14T14:59:56Z

`${new Date()
    .toISOString()
    .replace(/T.+/, "")}-${version}.md`

Ugly

fisker · 2020-07-14T15:06:08Z

`${new Date()
    .toISOString()
    .replace(/T.+/, "")}-${version}.md`

Ugly

This one seems not related. It's changed by #7889

#7889 Playground

brody2consult · 2020-07-14T15:08:30Z

Definitely looks like a positive change to me. Wouldn't this looks like a breaking change to some users?

thorn0 · 2020-07-14T17:04:27Z

${new Date()
This one seems not related. It's changed by #7889

Yes. I wonder what to do with it. Should we special-case Date to exempt it from the "always break constructor-call headed chains" rule or is it better to disable that rule inside template literals?

thorn0 · 2020-07-14T17:08:33Z

As for this PR, what do you all think about the reformatting of the mapDoc calls? E.g. here:
https://github.com/fisker/prettier/pull/429/files#diff-b943e07d378ae28b03b5ebd46e7e86ac
Does that look okay? I'm not sure.

thorn0 · 2020-07-14T17:14:26Z

@brodybits

Wouldn't this looks like a breaking change to some users?

Where exactly do you think this change might look unexpected?
Do you have any specific libraries/usage in mind?

brody2consult · 2020-07-14T17:38:36Z

Where exactly do you think this change might look unexpected?

If a massive set of changes would suddenly happen to a code base after a minor update of Prettier, like we would see in Prettier itself (fisker#429), I would be a bit surprised. Or am I too pedantic?

thorn0 · 2020-07-15T00:14:50Z

@brodybits I think people are okay with massive changes when these massive changes are perceived as in improvement.

fisker · 2020-07-19T10:17:29Z

what do you all think about the reformatting of the mapDoc calls?

I think it's good.

karptonite · 2020-07-21T17:39:21Z

@brodybits I think people are okay with massive changes when these massive changes are perceived as in improvement.

Recall also that, to some extent, this sort of rolls back another "massive" change that appeared in 1.19.

rassie · 2020-11-13T09:15:38Z

Any progress and/or resolution on this one? I've rebased this branch on master and tried running my codebase through it. It's somewhat better than the original, but there are still those weird single-line pipes, which used to be multi-line.

thorn0 · 2020-11-13T10:20:35Z

@rassie This PR wasn't meant to change the situation for RxJS pipes as I don't know a solution to it.

thorn0 · 2021-03-05T12:43:55Z

src/language-js/print/call-arguments.js

+  if (
+    hasComment(
+      lastArg,
+      CommentCheckFlags.Leading | CommentCheckFlags.Trailing


@fisker This doesn't work as expected. Something is wrong inside hasComment.

I was wrong. Actually, it makes sense. CommentCheckFlags.Leading | CommentCheckFlags.Trailing doesn't mean "leading or trailing". It means "leading and trailing".

Maybe we can add conflicting flags check? eg Leading/Trailing/Dangling and Block/Line

Yes, as an assertion removed at build time. Good idea.

…ike calls

thorn0 marked this pull request as draft May 13, 2020 13:44

thorn0 force-pushed the functional-composition-experiments branch 2 times, most recently from 5a12f60 to e14a67b Compare May 13, 2020 14:19

This was linked to issues May 14, 2020

arrow functions "glue" long lines at badly split places #4303

Open

Revisit function literal heuristic introduced in Prettier 1.19 #6921

Open

fisker mentioned this pull request Jun 30, 2020

Allow more arrow function print on same line with arrow #8673

Open

4 tasks

github-actions bot mentioned this pull request Nov 26, 2020

Run with pr 9672 fisker/prettier-regression-testing#2

Closed

Base automatically changed from master to main January 23, 2021 17:13

thorn0 force-pushed the functional-composition-experiments branch 2 times, most recently from 0ab08dc to e090f0a Compare March 5, 2021 11:16

github-actions bot mentioned this pull request Mar 5, 2021

pr 8300 -- Functional composition experiments prettier/prettier-regression-testing#42

Open

thorn0 commented Mar 5, 2021

View reviewed changes

An attempt to distinguish addEventListener-like and composeSelector-l…

11952f1

…ike calls

thorn0 force-pushed the functional-composition-experiments branch from e090f0a to 11952f1 Compare March 5, 2021 12:51

github-actions bot mentioned this pull request Mar 28, 2021

test sosukesuzuki/prettier-regression-testing#2

Open

Uh oh!

Conversation

thorn0 commented May 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fisker commented May 29, 2020

Uh oh!

fisker commented Jul 14, 2020

Uh oh!

alexander-akait commented Jul 14, 2020

Uh oh!

fisker commented Jul 14, 2020

Uh oh!

brody2consult commented Jul 14, 2020

Uh oh!

thorn0 commented Jul 14, 2020

Uh oh!

thorn0 commented Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thorn0 commented Jul 14, 2020

Uh oh!

brody2consult commented Jul 14, 2020

Uh oh!

thorn0 commented Jul 15, 2020

Uh oh!

fisker commented Jul 19, 2020

Uh oh!

karptonite commented Jul 21, 2020

Uh oh!

rassie commented Nov 13, 2020

Uh oh!

thorn0 commented Nov 13, 2020

Uh oh!

thorn0 Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

thorn0 Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

fisker Mar 5, 2021 • 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!

thorn0 Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

thorn0 commented May 13, 2020 •

edited

Loading

thorn0 commented Jul 14, 2020 •

edited

Loading

fisker Mar 5, 2021 •

edited

Loading