denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.07k stars 5.23k forks source link

deno fmt is too lax #7868

Open lucacasonato opened 3 years ago

lucacasonato commented 3 years ago

Here are some cases I have discovered, that I feel deno fmt is too lax about. What I mean with this, is that deno fmt likes to maintain what currently exists, over forcing a certain style. I think deno fmt should force a certain style more often.

This list is just a few examples. We should go through all dprint config options that are currently set to Maintain, and check if they should not be forcing a certain style instead. I think we should try to 1:1 match what prettier does.

Line breaks in function arguments

// Current options 

foo("foo bar", {
  why: "bar",
  bar: "foo",
});

foo(
  "foo bar",
  {
    why: "bar",
    bar: "foo",
  },
);

// Should always be

foo("foo bar", {
  why: "bar",
  bar: "foo",
});
// Current options

foo({
  foo: "bar",
  bar: "foo",
}, {
  why: "bar",
  bar: "foo",
});

foo(
  {
    foo: "bar",
    bar: "foo",
  },
  {
    why: "bar",
    bar: "foo",
  },
);

// Should always be

foo(
  {
    foo: "bar",
    bar: "foo",
  },
  {
    why: "bar",
    bar: "foo",
  }
);
// Current options

foo("foo", "bar", "foo", "nar");

foo(
  "foo",
  "bar",
  "foo",
  "nar",
);

// Should always be

foo("foo", "bar", "foo", "nar");
// Current
function name(
  item: string,
  foo: bar,
) {
}

// Should always be
function name(item: string, foo: bar) {}

For of loops

// Current
for (
  const iterator of {
    foo: bar,
  }
) {
}

// Should always be
for (const iterator of {
  foo: bar,
}) {
}
lucacasonato commented 3 years ago

I have discovered a good way to see this phenomenon is to go to repos formatted with deno fmt (e.g. https://github.com/denoland/deno_registry2) and run prettier on them, then run deno fmt again. There is a difference between old deno fmt and prettier, and differences between prettier and new deno fmt, and differences between old deno fmt and new deno fmt.

dsherret commented 3 years ago

Yeah, right now it's maintaining line breaks in many places. I wouldn't recommend turning on the "prefer single line" configuration yet as it still needs more work for some edge cases... and there are a lot of edge cases. I'm going to be slowly turning it on over time as things get better (ex. just recently turned it on for import and export declarations).

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

BlackAsLight commented 3 months ago

Is there any update in this area?