fix(eslint): use recommendedTypeChecked

[sacrosanctic]

Closes #938

Description

Checklist

• Update snapshots (if applicable)
• Add a changeset (if applicable)
• Allow maintainers to edit this PR
• I care about what I'm doing, no matter the tool I use (Notepad, Sublime, VSCode, AI...)

[pkg-svelte-dev]

Install the latest version of sv from d15b4f0:

pnx https://pkg.svelte.dev/sv/c/d15b4f0f23f6acb844fc7c29f6ef969a8fe1f0db create

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/cli/pr/1202

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

[changeset-bot]

⚠️ No Changeset found

Latest commit: d15b4f0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[sacrosanctic]

 ❯  addons  tests/eslint/test.ts (4 tests | 1 failed) 185289ms
   ✓ eslint 'kit-js'  113307ms
   ✓ eslint 'kit-ts'  113306ms
   ✓ eslint 'vite-js'  113306ms
   × eslint 'vite-ts' 173888ms (retry x3)

I don't understand how this project is structured. It has 2 tsconfig.json's.

[AdrianGonz97]

While looking into the lint failure for #1202 (comment), I started to go through the new lint rules (this list only contains what's added by recommendedTypeChecked to recommended).

I'm left wondering if the preset rules for recommendedTypeChecked may be too opinionated, or perhaps contain some questionable defaults?

For example, I can already foresee no-floating-promises becoming a problem when it comes to deferred query refreshes with remote functions.

something like this:

const foo = form(() => {
	// ...stuff happens
	
	query1().refresh() // lint error!
	query2().refresh() // lint error!
})

could erroneously lead people down the very wrong path of awaiting these refreshes:

const foo = form(() => {
	// ...stuff happens

	// oops! these no longer run concurrently!
	await query1().refresh()
	await query2().refresh()
})

however, while adding void query1().refresh() may dismiss the lint error, that 'fix' will not be everyone's first instinct.

only-throw-error could be an issue with thrown a redirect, though i haven't tested it so i can't be certain at the moment.

I also vaguely remember require-await being the source of some annoying issues when we first tried to enable it for this repo, but I'm not able to recall exactly why. Do you guys (@manuel3108, @benmccann) happen to remember?

[sacrosanctic]

Good stuff, definitely things to consider, hopefully this PR can create the discussion needed to merge this or to close the original issue. Will have to do more exploration with the new APIs.

however, while adding void query1().refresh() may dismiss the lint error, that 'fix' will not be everyone's first instinct.

It is part of the lint error, maybe we can overwrite it with our own custom error msg?

only-throw-error could be an issue with thrown a redirect

Don't think this is an issue, we don't throw error or redirect.

[manuel3108]

I also vaguely remember require-await being the source of some annoying issues when we first tried to enable it for this repo, but I'm not able to recall exactly why. Do you guys (@manuel3108, @benmccann) happen to remember?

Sadly i don't remember any of this. But I see your points.

Do I see that correctly, that this PR tries to change the setup for new projects (sv create) and our own codebase? I mean it totally makes sense, just trying to understand the scope of this PR

[manuel3108]

Changed target branch and resolved produced merge conflicts

[manuel3108]

What is the status of this? Is this something we should look into now?

[sacrosanctic]

I don't know if you guys want to go through with it. The PR represents the required changes. Plus one more issue with node in the vite-ts template that I haven't resolved.

The PR is also stale, will try to bring it up to date.

[sacrosanctic]

omg, all the tests passed, how did that happen

[sacrosanctic]

@manuel3108 This pr is ready to go. But @AdrianGonz97 mentions some issues for the upcoming async svelte with regards to conflicting eslint rules. We may need to fix them by providing our own modified versions, similar to svelte/prefer-const.

@benmccann pinging previously mentioned parties