Update ESLint and add Stylistic Formatting

[KristjanESPERANTO]

Prettier and ESLint are a nice combination.

Update: Because prettier hardly allows any customization, we switched to stylistic in the process of the review.

[socket-security]

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@stylistic/[email protected]	Transitive: environment, eval, filesystem, shell, unsafe	+118	55.6 MB	antfu

View full report↗︎

[KristjanESPERANTO]

The tests are failing because I didn't run npm run lint:fix intentionally.

[derhuerst]

A appreciate the intention to make the code's formatting consistent and reproducible, but infortunately in the past years, whenever I have encountered prettier-formatted code, prettier's formatting style was really incompatible with the line-based nature of Git and trying to minimise unrelated changes in commits.

I'm aware that I'm letting technical limitations of Git dictate my code style here, but given that I'm bound to use Git, the benefits of having Git-diff-friendly code formatting are significant: Less conflicts while rebasing/merging, more meaningful git blame to remember why a change was made, etc.

The problem that I have with automated formatting: It depends on a specific code section's business logic how likely it is to be extended in the future (e.g. an another argument added to a function call, or another field to an object), and the current tools cannot estimate that. For example a addNumbers(a, b) call is unlikely to be extended in the future, a sendRequest(url, body, headers) much more.

If we accept a "dumb" and therefore inherently Git-diff-unfriendly code formatting tool, let's pick one – or a well-supported way to get prettier to do this – that does the following:

• always use trailing commas in arrays, arguments lists, etc.
• long arithmetic expressions, ternary expressions, arguments lists, arrays of non-primitives, objects, destructuring assignments, export expressions, etc: almost always on multiple lines instead of one, to make future changes have nicer Git diffs
• always use curly braces for multi-line blocks (if/else, while, etc)
• when >1 function calls are chained, never inline them, but always give each call a separate line
• don't inline multi-line empty objects if they are nested in a data structure (e.g. here)

[KristjanESPERANTO]

Thank you for the detailed explanation! This gives me a new perspective and I'm thinking about adapting my usual workflow.

Since prettier has very few options, I have chosen a different approach with the last commit. stylistic is much more adjustable than prettier. What do you think, is it worth going further in this direction?

[KristjanESPERANTO]

As example I added the linted/formatted index.js.

[derhuerst]

The linting setup currently produces output like this:

const locHints = (l.remarkRefs || []).
	filter(ref => !!ref.hint && Array.isArray(ref.tagL)).
	filter(({tagL}) => tagL.includes('RES_LOC') ||
	tagL.find(t => t.slice(
		0,
		8,
	) === 'RES_LOC_'), // e.g. `RES_LOC_H3`
	// eslint-disable-next-line @stylistic/function-paren-newline
	).
	map(ref => ref.hint)

Following the goals I outlined before, I would like it to look as follows:

const locHints = (l.remarkRefs || [])
	.filter(ref => !!ref.hint && Array.isArray(ref.tagL))
	.filter(({tagL}) => (
		tagL.includes('RES_LOC') ||
		tagL.find(t => t.slice(0, 8) === 'RES_LOC_' // e.g. `RES_LOC_H3`
	))
	.map(ref => ref.hint)

(I wouldn't really mind the first .filter() call being broken up into multiple lines; Same for the .find() call. But trust people (contributors and me) to decide case-by-case, and would therefore turn off the rule about this.)

---

Sorry to put the tedious task of configuring a linter on you…

[derhuerst]

I dislike the style, but for the sake of consistency, can we get all of thee arguments indented and with a trailing comma?

[KristjanESPERANTO]

Now it looks like that:

			includeRelatedStations: true,
		}, opt)

Do you want a trailing comma after opt?

[KristjanESPERANTO]

I see it as a learning process, don't worry.

Sorry, I just realized that I have to go back a few steps. Please wait with the review.

[KristjanESPERANTO]

I think I'm a bit closer to this what you want, but now I have to figure out why the tests are failing here. Locally they are not failing. I'll try to continue in the next few days.

[KristjanESPERANTO]

Okay, the unit tests are now running 🚀 and I think we are now close to your specifications. I'm delighted that stylistic is so customizable 😃 There's certainly still potential for some fine-tuning.

Would you like to take a look? There are changes to only 171 files 😅

[derhuerst]

I'm happy with almost all formatting! Just some small items left.

[derhuerst]

Because I couldn't find a better way to contact you, so I'll ask here, independently of this PR: What do you think about becoming a co- oder fallback maintainer (up to you which) in case I end up not being able to take care of hafas-client anymore? If you have more questions or comments on this, don't hesitate to contact me, e.g. via email!

[derhuerst]

Thanks!

[KristjanESPERANTO]

🥳