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

turn off "import/no-unresolved" #15

Merged
merged 1 commit into from
Jul 21, 2021
Merged

Conversation

tanhauhau
Copy link
Member

unnecessary, unresolved modules, most likely caught by typescript check

and this will cause us to "adding legacy cruft to modules"

@benmccann
Copy link
Member

Need to remove eslint-plugin-import from package.json as well I believe

Unfortunately Rollup will not catch these errors, so they could still happen in SvelteKit since it doesn't use TypeScript. I think it will create a warning in Rollup, but we already have a handful of warnings, so really easy to miss. But we may be able to turn it on independently there, so this is probably fine to merge here

@tanhauhau
Copy link
Member Author

just to confirm, removing eslint-plugin-import will remove all the other rules comes with eslint-plugin-import, not just import/unresolved. (it comes with some stylistic lint rules) https://github.com/benmosher/eslint-plugin-import#style-guide

so we don't need linting rules from eslint-plugin-import right?

@benmccann
Copy link
Member

Ah, ok. I didn't realize that. Let's leave it then

@benmccann benmccann merged commit 1eaa6c5 into sveltejs:master Jul 21, 2021
@dummdidumm
Copy link
Member

Unfortunately Rollup will not catch these errors, so they could still happen in SvelteKit since it doesn't use TypeScript. I think it will create a warning in Rollup, but we already have a handful of warnings, so really easy to miss. But we may be able to turn it on independently there, so this is probably fine to merge here

I don't think so, part of the CI checks is running a type check with TSC on the files, and those check JS, too, so they should catch that.

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

Successfully merging this pull request may close these issues.

3 participants