Formatting with prettier

[ghost]

Currently, this project includes a .prettierrc, there are a couple of issues with this...

First, the small one:

It appears the prettier config is a YAML file. Though, according to the prettier docs, it is valid for YAML configs to omit the file extension, VS Code incorrectly recognizes the file as JSON. Therefore, it's probably a good idea to include a file extension. Either way, YAML seems a little out of place. Maybe rewrite it as a .json or .js file?

Second, the bigger and real issue:

Prettier is awesome IMO and I think it's great that this project uses it...except for the fact that the project doesn't appear to be using it LOL. My code editor is configured to format with prettier on save. Saving a file revealed that prettier had never been run on it. Further investigation revealed:

👻 ➜  svelte git:(master) ✗ npx prettier --check src/**/*.ts

npx: installed 1 in 1.48s
Checking formatting...
src/compiler/Stats.ts
...
...(probably every other file)
...
src/runtime/store/index.ts
src/runtime/transition/index.ts
Code style issues found in the above file(s). Forgot to run Prettier?

So, I think it's a good idea to run prettier once on the entire codebase and then on every subsequent pull request. I'm happy to do so and submit a PR but that might make me the single largest contributor to Svelte 😆. Plus, who would review that? So, I think formatting the entire codebase should be done by that red-headed guy who suggested it: #508 (comment).

[ghost]

Yea, just ran prettier to see. 1,090 lines changed, so far from 1st place, but still top 10!

[Conduitry]

When Prettier was initially introduced in this codebase two and a half years ago, the tooling for ESLint with TypeScript was in a much different place than it is now. And the last I checked, there were also a bunch of code related to generating code that Prettier actually made a lot more confusing looking. I'd actually be in favor of dropping Prettier entirely and in its place adding a few stylistic ESLint rules. Another alternative might be to significantly increase the maximum line length in Prettier. There are a lot of template strings involved with codegen that Prettier wanted to split (in a strange way) onto multiple lines. I agree that something has to be done, but whether that something involves keeping Prettier I do not know.

[ghost]

Stop breaking simple template literals (prettier/prettier#5979 by @jwbay)

This is one of the Prettier parts that get a lot of change requests: breaking simple expressions inside template literals. Previously Prettier would break the expressions in multiple lines if the whole literal exceeded the print width. Now, we'll prevent breaking if the expression is simple.

I don't know if that changes anything but yea, something should be done, even if that means ditching prettier, because right now some files are formatted with it while others aren't. Pull requests will be a mixed bag. Simply running prettier across the whole codebase will break things. In at least one case, it relocated an eslint ignore comment, which either split the code in two or enabled eslint to throw an error. Then I ran the tests and many more things broke.

In most cases however, I think the benefit of prettier out weighs the alternative: an inconsistent codebase. If we can safely get the entire codebase formatted and thereafter use something like lint-staged, as well as make some concrete style decisions, I think that's the best way to go.

There is no style that will please everyone—at least at first—but consistency gives us something to adapt to. I always hated trailing commas, until I realized they enabled me to use a plugin to alphabetize object properties without having to rearrange the commas. And now I can never go back. Is that overboard?

[Tobbe]

a plugin to alphabetize object properties

OMG, I had no idea that was a thing! I'm already loving it! Where can I find it?

[ghost]

I don't know if it is a thing. I use alphabetical-sorter for vscode. You highlight a block of text then cmd/ctrl + shift + a and there you have it. It's also great for CSS properties and dependencies in your package.json.