dprint / dprint-plugin-typescript

TypeScript and JavaScript code formatting plugin for dprint.
https://dprint.dev/plugins/typescript
MIT License
255 stars 57 forks source link

Add new configs to improve memberExpressions #317

Open declanvong opened 2 years ago

declanvong commented 2 years ago

There are some cases where the combination memberExpression.preferSingleLine: false, memberExpression.linePerExpression: true doesn't produce a nice format. My goal is to produce something similar to what Prettier does with method chaining, which is a little smarter about it. preferSingleLine is false, so that you don't end up with something like:

this.is.a.method().chain()
  .with().prefer().single()
  .line().true();

and linePerExpression so that we get the .foo() on its own line, instead of attaching to the previous node:

yargs
  .option({
    // ...
  })
  .option({
    // ...
  });

So, with those configurations, there are two things I'm wondering if there can be improvements for. Not sure if it's best for them to be unconditional or behind a config.

1. Chaining should only happen on methods:

Input:

document.pages
.map(p => p.elements)
.filter(e => e != null && e.length)
.sort((e1, e2) => e1.length - e2.length);

Expected + what Prettier outputs:

document.pages
  .map(p => p.elements)
  .filter(e => e != null && e.length)
  .sort((e1, e2) => e1.length - e2.length);

Output:

document
  .pages
  .map(p => p.elements)
  .filter(e => e != null && e.length)
  .sort((e1, e2) => e1.length - e2.length);

It'd be preferable if the member expression for .pages was still attached to the root.

2. linePerExpression leads to unnecessary hanging lines when trailing from an array or ParenExpr:

Input:

const a = [
  getFoo(),
  getBar(),
].filter(Boolean);

Expected + what Prettier outputs:

const a = [
  getFoo(),
  getBar(),
].filter(Boolean);

Output:

const a = [
  getFoo(),
  getBar(),
]
  .filter(Boolean);

I think it can probably avoid the newline if it's the only member expr.

For some reason the Playground doesn't work for me at the moment (not sure if my internet, or if it's actually down), so can't provide links there, sorry.

declanvong commented 2 years ago

Happy to raise a PR for this one if it aligns with your views and you think it's a step in the right direction -- otherwise I'm also happy to just keep these in our fork.

declanvong commented 2 years ago

Our fork now has a small customisation memberExpression.linePerExpression: methods (naming is not great...) which resolves this. It essentially does something similar to Prettier here:

== should break method expressions onto new lines when exceeding the width ==
testing.this.out.for.when().it.passes.the.lineWidth().a.b;

variableA.variableB.methodCallA().map(x => x.y.z).a.b.c.reduce((a, c) => foo, {}).build();

[expect]
testing.this.out.for
    .when()
    .it.passes.the.lineWidth().a.b;

variableA.variableB
    .methodCallA()
    .map(x => x.y.z)
    .a.b.c.reduce((a, c) => foo, {})
    .build();

If the whole expression exceeds the line width: 1) Add a newline before the first call expression 2) Add a newline after each call expression, except... 3) Any non-call expressions that are trailing after the last call expression will try to just fit onto that line if possible.

Very happy to raise a PR with this if that seems reasonable to you! Might have to figure out a better name though 😂 I didn't put much thought into it as I'm getting this in for a big reformat that I'm applying over the Christmas break while most of our developers are away on leave, to avoid conflicts as much as possible.

declanvong commented 2 years ago

Hm, I'm not sure why I actually closed this given that it's still a problem here in upstream. @dsherret are you able to reopen this one?