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

Interoperate with Existing ESLint Rules and Configs #25

Open
chances opened this issue Apr 8, 2020 · 8 comments
Open

Interoperate with Existing ESLint Rules and Configs #25

chances opened this issue Apr 8, 2020 · 8 comments

Comments

@chances
Copy link

chances commented Apr 8, 2020

@axetroy If you say there are too many options, why build out a deno lint at all? That's only adding more options. We already have a linting tool for linting JavaScript and TypeScript and it is called eslint. I don't see a value in duplicating all that work again.

Unless a developer only writes backend code, they will still need to touch eslint.

If a parallel lint tool must be built (are there really not better ways to improve deno?) then it MUST be able to consume existing eslint rules. Otherwise people will waste their time reinventing the wheel, writing duplicate rules for deno and frontend code rather than application code.

Instead deno lint would be more useful as something like golangci-lint where it aggregates and calls other already written lint rules and linters. It provides a "default lint config" (via url import) with sane default but still extendable.

Originally posted by @brandonkal in denoland/deno#1880 (comment)

Emphasis added.

@nayeemrmn
Copy link
Contributor

What if deno lint worked for frontend code?

@axetroy
Copy link

axetroy commented Apr 8, 2020

My opinion is that linter should be fewer configurations (even 0)

The most popular eslint rules should be adopted, but no options are provided.

In the short term, this is very bad, because everyone's writing habits are different

But in the long term, it unifies the code style

As I think about deno fmt

We should not add some flags to set the code style, eg. whether use trailing commas, whether to use spaces or tabs. I think this is very bad.

People don’t have to discuss which one is the best style

@Soremwar
Copy link

Soremwar commented Apr 8, 2020

Code style is only limited by your project and your personal writing style as you said. But having no option to thinker with that options only removes value to an otherwise highly (though opinionated by default) customizable environment

As has been discussed before, Deno doesn't do magic, doesn't pick default files and prefers to allow customization even though its defaults will be highly influenced by products such as ESLint and Prettier

@trevordmiller
Copy link

trevordmiller commented Apr 24, 2020

I would suggest having the linting setup include only common problems only; perhaps extending eslint:recommended (which is common problems only), something like:

{
    "parserOptions": {
      "ecmaVersion": X
    },
    "extends": [
      "eslint:recommended"
    ]
}

@bartlomieju
Copy link
Member

Discussed offline with @magurotuna, we agree this is highly desirable and will look into what are the prerequisites.

@RobertAKARobin
Copy link

RobertAKARobin commented Nov 8, 2022

Deno's linter doesn't provide alternatives for all the Eslint rules we've historically used, which is a deal-breaker for us switching to Deno.

We tried adding workarounds to Eslint to let it run in Deno files (see typescript-eslint/typescript-eslint#5921), but it's overly complicated and kind-of outside the scope of Eslint. IMO it would really behoove Deno Lint to natively support Eslint rules, rather than Deno Lint trying to come up with its own replacements for Eslint rules.

AFAIK there are a few things that prevent Eslint from working with Deno:

Eslint can't resolve .ts extensions

That's easy to work around:

// ts-resolver.js
const ts = require(`typescript`);
const tsExtension = /\.ts$/;
module.exports = {
	resolveModuleNames: (
		moduleNames,
		containingFile,
		reusedNames,
		redirectedReference,
		options
	) => moduleNames.map((moduleName) =>
		ts.resolveModuleName(
			moduleName.replace(tsExtension, ``),
			containingFile,
			options,
			ts.sys
		).resolvedModule
	),
};
// .eslintrc.js
module.exports = {
	env: {
		node: true,
	},
	overrides: [
		{
			files: [`*.ts`],
			parser: `@typescript-eslint/parser`,
			parserOptions: {
				moduleResolver: __dirname + `/ts-resolver.js`,
			},
		},
	],
};

Eslint can't resolve Deno's remote dependencies

For example:

import { serve } from 'https://deno.land/std@0.133.0/http/server.ts';

The eslint-import-resolver-deno shows how to work around this, and it's not complicated. The package can also just be used wholesale:

const ts = require(`typescript`);
const denoResolver = require(`eslint-import-resolver-deno`);

const tsExtension = /\.ts$/;
module.exports = {
	resolveModuleNames: (
		moduleNames,
		containingFile,
		reusedNames,
		redirectedReference,
		options
	) => moduleNames.map((moduleName) => {
		if (moduleName.startsWith(`http`)) {
			const { found, path } = denoResolver.resolve(
				moduleName,
				containingFile,
				{
					importMap: __dirname + `/import-map.json`,
				}
			);
			if (found) {
				moduleName = path;
			}
		}
		return ts.resolveModuleName(
			moduleName.replace(tsExtension, ``),
			containingFile,
			options,
			ts.sys
		).resolvedModule;
	}),
};

Eslint doesn't "know" about Deno's global types

Despite the above workarounds to the module resolution issues, Eslint interprets anything reliant on a global Deno type as any.

If Eslint can be configured to know about Deno's global types, then it seems like Eslint could support Deno pretty easily. Maybe this in turn means that Deno Lint could support Eslint pretty easily?

@RobertAKARobin
Copy link

Update: Well, I'm getting there. I have ESlint itself apparently working. But I can't make the VSCode ESlint extension play nice with Deno imports.

@dandv
Copy link

dandv commented Jun 12, 2024

Related discussion: #305 (comment)

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

No branches or pull requests

8 participants