Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checking promise-related errors #37

Open
jrmajor opened this issue Apr 11, 2024 · 2 comments
Open

Checking promise-related errors #37

jrmajor opened this issue Apr 11, 2024 · 2 comments

Comments

@jrmajor
Copy link

jrmajor commented Apr 11, 2024

Regarding #31 (comment) (cc @benmccann):

Something else I was interested in trying was checking for promise-related errors. I just noticed in another project that TypeScript doesn't catch these types of errors. It should probably be another PR and isn't related to this change at all, but I thought I'd mention it just in case you were interested in working on it at all:

immich-app/immich@7f504ec/web/.eslintrc.cjs#L54

These rules report an error for every file that doesn't have a tsconfig.json. Not only we would need to enable them only for subdirectories that have a tsconfig.json, but we would need to exclude paths that are not included/are excluded in tsconfig.json. Here's how it looks for the main Svelte repo:

{
	files: [
		// only for packages/svelte
		// we would need to specify more paths to make it work for sites/*
		// it needs to be kept in sync with include/exclude paths in tsconfigs
		'packages/svelte/src/**/*',
		'packages/svelte/tests/*/test.ts',
		'packages/svelte/tests/runtime-browser/test-ssr.ts',
		'packages/svelte/tests/*/samples/*/_config.js'
	],
	languageOptions: {
		parserOptions: {
			project: true
		}
	},
	rules: ts.configs.recommendedTypeCheckedOnly[2].rules,
},
{
	rules: {
		// too many errors reported
		// without these, there are 11 errors
		'@typescript-eslint/no-unsafe-argument': 'off',
		'@typescript-eslint/no-unsafe-assignment': 'off',
		'@typescript-eslint/no-unsafe-call': 'off',
		'@typescript-eslint/no-unsafe-member-access': 'off',
		'@typescript-eslint/no-unsafe-return': 'off',
		'@typescript-eslint/unbound-method': 'off'
	}
},

Because of that, we can't just turn these rules on in @sveltejs/eslint-config. We could expose a helper for configuring them, I don't think it's worth it though:

/** @type {import('eslint').Linter.FlatConfig[]} */
export default [
	...svelteConfig,
	...svelteConfigTypeChecked([
		'packages/svelte/src/**/*',
		'packages/svelte/tests/*/test.ts',
		'packages/svelte/tests/runtime-browser/test-ssr.ts',
		'packages/svelte/tests/*/samples/*/_config.js'
	])
];

I can also send PRs directly to sveltejs/svelte and sveltejs/kit repos.

@benmccann
Copy link
Member

Some of these rules have been setup in sveltejs/svelte in sveltejs/svelte#11842

@benmccann
Copy link
Member

Enabled some ts stuff in SvelteKit now too: sveltejs/kit#12297

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants