Use Prettier for ESLint formatting TypeScript #23631

nschonni · 2023-02-16T06:48:19Z

Adds the ESLint prettier configs and removes rules shadowed by that plugin https://github.com/prettier/eslint-config-prettier/blob/main/index.js
Ran the noisy cleanup as part of the second commit

nschonni · 2023-02-17T09:48:23Z

I think the CodeQL failure is because the previously suppressed warnings are popping up because the suppression through the UI are line specific

ykzts · 2023-02-18T14:37:44Z

Mastodon aligns the = position of the assignment with a space; Prettier removes this space to one, so we do not use Prettier in JS. We should be able to ignore this formatting in the Prettier options.

github-actions · 2023-02-19T06:44:02Z

This pull request has merge conflicts that must be resolved before it can be merged.

nschonni · 2023-02-19T07:06:52Z

Mastodon aligns the = position of the assignment with a space; Prettier removes this space to one, so we do not use Prettier in JS. We should be able to ignore this formatting in the Prettier options.

Prettier is an opinionated formatter with few options https://prettier.io/docs/en/options.html
Since it usually aims to minimize diffs, things like having to reflow lines to realign with the equals sign wouldn't likely become an option there

github-actions · 2023-02-20T02:40:59Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2023-02-21T09:00:12Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2023-02-21T16:31:36Z

This pull request has resolved merge conflicts and is ready for review.

github-actions · 2023-02-22T01:17:01Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2023-02-22T01:23:25Z

This pull request has resolved merge conflicts and is ready for review.

github-actions · 2023-02-24T19:25:24Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2023-02-24T20:29:11Z

This pull request has resolved merge conflicts and is ready for review.

nschonni · 2023-02-24T20:29:42Z

I split the commits between the JSX and JS files to see if one or the other might be easier to accept initially

github-actions · 2023-02-26T19:30:24Z

This pull request has merge conflicts that must be resolved before it can be merged.

.prettierignore

app/javascript/mastodon/is_mobile.ts

renchap · 2023-05-04T07:08:22Z

Looks good!

Should we update lint-staged so it only runs prettier on known extensions, ignoring (t|j)sx?? Currently, it will run both prettier --write and eslint --fix on JS/TS files, which may both update the file, and I fear this can create issues.

What about this? Could this cause issues?

nschonni · 2023-05-04T07:11:03Z

No, you can run yarn prettier -w "**/*.ts" and there is no changes to the files or warnings from ESLint after it did the autofixing

renchap

👍

Let's merge this!

ClearlyClaire · 2023-05-04T10:42:57Z

app/javascript/mastodon/components/avatar_overlay.tsx

+  const { hovering, handleMouseEnter, handleMouseLeave } =
+    useHovering(autoPlayGif);


Is that way of having the right-hand side entirely on a new line really more readable?

The whole line is > 80 char, so Prettier reformats it to be under this limit by moving the right-hand side to the new line

Yeah I understand this is a result of applying the > 80 chars rule, I just find that less readable. But that's a pretty minor corner-case.

ClearlyClaire · 2023-05-04T10:43:47Z

app/javascript/mastodon/components/avatar_overlay.tsx

-      className='account__avatar-overlay' style={{ width: size, height: size }}
+      className="account__avatar-overlay"


Afaik, this quote style is the opposite of the code style we've been enforcing since the beginning of the project.

This can be changed using the jsxSingleQuote option if needed, but I think double quotes are the standard nowadays in JSX, as they are in HTML.

ClearlyClaire · 2023-05-04T10:50:52Z

app/javascript/mastodon/components/relative_timestamp.tsx

-    const { timestamp }  = props;
-    const delta          = (new Date(timestamp)).getTime() - state.now;
-    const unitDelay      = getUnitDelay(selectUnits(delta));
-    const unitRemainder  = Math.abs(delta % unitDelay);
+    const { timestamp } = props;
+    const delta = new Date(timestamp).getTime() - state.now;
+    const unitDelay = getUnitDelay(selectUnits(delta));
+    const unitRemainder = Math.abs(delta % unitDelay);


Afaik this is another change from a coding style we were leaning towards. I don't have a strong opinion either way, but I'm wary of changing existing rules, especially if we end up applying this to JS files as well.

I think one of the goals that prettier seems to aim for is to minimize the git noise. Since if the longest variable changes length, the rest of the lines would need to be re-written, so there is no option to use this style with prettier.

That does make sense. I'd just want to have @Gargron to ack on this has he has explicitly asked for the current code style in the past (when our toolchain did not enforce either code style).

renchap · 2023-05-08T13:15:39Z

I would really like this to be merged soon as more Typescript is appearing in the codebase.

@nschonni could you enable jsxSingleQuote as this is the current style in the codebase, rebase it, and reformat the newly-added TS files (if needed)?

Do you have any other remarks we need to act on @ClearlyClaire?

github-actions · 2023-05-09T01:29:33Z

This pull request has merge conflicts that must be resolved before it can be merged.

ClearlyClaire · 2023-05-09T10:06:23Z

With my glitch-soc maintainer hat, in addition to my earlier comment, I'd like to avoid bundling automatic changes with manual changes (e.g. removing the iOS detection thing).

The reason is glitch-soc works by maintaining a copy of the front-end files with local modifications, and trying to apply a large-scale change like automatic linting is most likely going to not work, but if I know this is just automatic linting, I can run the linter separately. However, if the commit contains both automatic changes and manual changes, I cannot do that (and Mastodon PRs end up squashed in a single commit, so that would end up as a single commit).

Extracted from @nshonni's work here: mastodon#23631

github-actions · 2023-05-09T16:38:20Z

This pull request has resolved merge conflicts and is ready for review.

renchap · 2023-05-09T17:10:47Z

Thanks a lot for your work on this @nschonni ❤️

github-actions bot added the rebase needed 🚧 label Feb 19, 2023

nschonni force-pushed the eslint-prettier branch from 07f000e to 1be3f33 Compare February 19, 2023 07:04

github-actions bot removed the rebase needed 🚧 label Feb 19, 2023

github-actions bot added the rebase needed 🚧 label Feb 20, 2023

nschonni force-pushed the eslint-prettier branch from 1be3f33 to 129ae48 Compare February 20, 2023 03:14

github-actions bot removed the rebase needed 🚧 label Feb 20, 2023

nschonni force-pushed the eslint-prettier branch from 129ae48 to 3451a24 Compare February 20, 2023 07:18

github-actions bot added the rebase needed 🚧 label Feb 21, 2023

nschonni force-pushed the eslint-prettier branch from 3451a24 to 10abf35 Compare February 21, 2023 16:31

github-actions bot removed the rebase needed 🚧 label Feb 21, 2023

github-actions bot added the rebase needed 🚧 label Feb 22, 2023

nschonni force-pushed the eslint-prettier branch from 10abf35 to a726821 Compare February 22, 2023 01:23

github-actions bot removed the rebase needed 🚧 label Feb 22, 2023

github-actions bot added the rebase needed 🚧 label Feb 24, 2023

nschonni force-pushed the eslint-prettier branch from a726821 to 5659d80 Compare February 24, 2023 20:28

github-actions bot removed the rebase needed 🚧 label Feb 24, 2023

github-actions bot added the rebase needed 🚧 label Feb 26, 2023

renchap reviewed May 3, 2023

View reviewed changes

.prettierignore Show resolved Hide resolved

app/javascript/mastodon/is_mobile.ts Outdated Show resolved Hide resolved

nschonni force-pushed the eslint-prettier branch from 332208e to 0476861 Compare May 4, 2023 04:49

renchap approved these changes May 4, 2023

View reviewed changes

ClearlyClaire self-requested a review May 4, 2023 10:34

renchap mentioned this pull request May 4, 2023

Rewrite actions/modal and reducers/modal with typescript #24833

Merged

ClearlyClaire reviewed May 4, 2023

View reviewed changes

nschonni force-pushed the eslint-prettier branch from 0476861 to 682a392 Compare May 8, 2023 18:26

github-actions bot added the rebase needed 🚧 label May 9, 2023

renchap added a commit to renchap/mastodon that referenced this pull request May 9, 2023

Remove unused iOS agent sniffing function

50fe98a

Extracted from @nshonni's work here: mastodon#23631

renchap mentioned this pull request May 9, 2023

Remove unused iOS agent sniffing function #24931

Merged

nschonni added 3 commits May 9, 2023 12:34

Use Prettier for ESLint formatting TypeScript

73e65a3

Autoformat TS files with ESLint/Prettier

51159c1

Autorefactor to template string

d072e88

nschonni force-pushed the eslint-prettier branch from 682a392 to d072e88 Compare May 9, 2023 16:38

github-actions bot removed the rebase needed 🚧 label May 9, 2023

renchap requested a review from ClearlyClaire May 9, 2023 16:47

Gargron approved these changes May 9, 2023

View reviewed changes

Gargron merged commit 51b83ed into mastodon:main May 9, 2023
29 checks passed

nschonni deleted the eslint-prettier branch May 9, 2023 17:03

taichi221228 pushed a commit to taichi221228/mastodon that referenced this pull request May 16, 2023

Use Prettier for ESLint formatting TypeScript (mastodon#23631)

676bae4

skerit pushed a commit to 11ways/mastodon that referenced this pull request Jul 7, 2023

Use Prettier for ESLint formatting TypeScript (mastodon#23631)

b26e6a7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Prettier for ESLint formatting TypeScript #23631

Use Prettier for ESLint formatting TypeScript #23631

nschonni commented Feb 16, 2023

nschonni commented Feb 17, 2023

ykzts commented Feb 18, 2023

github-actions bot commented Feb 19, 2023

nschonni commented Feb 19, 2023

github-actions bot commented Feb 20, 2023

github-actions bot commented Feb 21, 2023

github-actions bot commented Feb 21, 2023

github-actions bot commented Feb 22, 2023

github-actions bot commented Feb 22, 2023

github-actions bot commented Feb 24, 2023

github-actions bot commented Feb 24, 2023

nschonni commented Feb 24, 2023

github-actions bot commented Feb 26, 2023

renchap commented May 4, 2023

nschonni commented May 4, 2023

renchap left a comment

ClearlyClaire May 4, 2023

renchap May 4, 2023

ClearlyClaire May 9, 2023

ClearlyClaire May 4, 2023

renchap May 4, 2023

ClearlyClaire May 4, 2023

nschonni May 4, 2023 •

edited

ClearlyClaire May 9, 2023

renchap commented May 8, 2023

github-actions bot commented May 9, 2023

ClearlyClaire commented May 9, 2023

github-actions bot commented May 9, 2023

renchap commented May 9, 2023 •

edited

		const { hovering, handleMouseEnter, handleMouseLeave } =
		useHovering(autoPlayGif);

		className='account__avatar-overlay' style={{ width: size, height: size }}
		className="account__avatar-overlay"

Use Prettier for ESLint formatting TypeScript #23631

Use Prettier for ESLint formatting TypeScript #23631

Conversation

nschonni commented Feb 16, 2023

nschonni commented Feb 17, 2023

ykzts commented Feb 18, 2023

github-actions bot commented Feb 19, 2023

nschonni commented Feb 19, 2023

github-actions bot commented Feb 20, 2023

github-actions bot commented Feb 21, 2023

github-actions bot commented Feb 21, 2023

github-actions bot commented Feb 22, 2023

github-actions bot commented Feb 22, 2023

github-actions bot commented Feb 24, 2023

github-actions bot commented Feb 24, 2023

nschonni commented Feb 24, 2023

github-actions bot commented Feb 26, 2023

renchap commented May 4, 2023

nschonni commented May 4, 2023

renchap left a comment

Choose a reason for hiding this comment

ClearlyClaire May 4, 2023

Choose a reason for hiding this comment

renchap May 4, 2023

Choose a reason for hiding this comment

ClearlyClaire May 9, 2023

Choose a reason for hiding this comment

ClearlyClaire May 4, 2023

Choose a reason for hiding this comment

renchap May 4, 2023

Choose a reason for hiding this comment

ClearlyClaire May 4, 2023

Choose a reason for hiding this comment

nschonni May 4, 2023 • edited

Choose a reason for hiding this comment

ClearlyClaire May 9, 2023

Choose a reason for hiding this comment

renchap commented May 8, 2023

github-actions bot commented May 9, 2023

ClearlyClaire commented May 9, 2023

github-actions bot commented May 9, 2023

renchap commented May 9, 2023 • edited

nschonni May 4, 2023 •

edited

renchap commented May 9, 2023 •

edited