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

Also lint CSS #835

Open
jaydenseric opened this issue Sep 3, 2021 · 23 comments
Open

Also lint CSS #835

jaydenseric opened this issue Sep 3, 2021 · 23 comments
Labels
request Feature request

Comments

@jaydenseric
Copy link

It would be great if Deno could also lint CSS in .css files, both via the deno lint CLI but also via the Deno LSP, which at the moment is capable of formatting css syntax.

The default CSS lint rules should basically just validate the CSS; there should be no formatting related rules as that's handled by Deno fmt.

Ideally there would also be rules to validate and auto fix CSS vendor-prefixes, against browserslist config.

In the meantime, I'm not sure the best way to handle linting of CSS in a Deno project without introducing a package.json file with dev dependencies such as stylelint installed and run via npm 🤮

@kitsonk kitsonk added the request Feature request label Sep 3, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Sep 3, 2021

Because Deno doesn't handle CSS natively, or even import it, it seem like it is a scope issue. It is a lot more than waving a magic wand and linting CSS. It requires a parser and an opionated set of style rules, neither one Deno has any opinions about.

Personally I would be opposed up until the point where Deno imports and processes CSS, which with the current state of the discussion with TC39 and the wider platform, that won't be anytime soon.

@bartlomieju
Copy link
Member

I agree with Kitson. Deno is completely agnostic of CSS and can't use it in anyway.

Personally I would be opposed up until the point where Deno imports and processes CSS, which with the current state of the discussion with TC39 and the wider platform, that won't be anytime soon.

Agreed, this request is currently out of scope for deno_lint and I'm not keen on adding CSS infrastructure.

@jthegedus
Copy link

jthegedus commented Sep 8, 2021

I came here not for CSS, but expecting deno lint to support the same set of files that deno fmt supports. I understand these are built on separate underlying rust infra, however there is no clear distinction of the goals with these two cmds, one would expect their goals/coverage to align.

@bartlomieju
Copy link
Member

I came here not for CSS, but expecting deno lint to support the same set of files that deno fmt supports. I understand these are built on separate underlying rust infra, however there is no clear distinction of the goals with these two cmds, one would expect their goals/coverage to align.

So you expect deno_lint to lint Markdown and JSON/JSONC files?

@kitsonk
Copy link
Contributor

kitsonk commented Sep 8, 2021

What rule set would you propose for linting Markdown and JSON/JSONC?

@jaydenseric
Copy link
Author

jaydenseric commented Sep 8, 2021

What we don't want, is the linting wheels to be reinvented again and again like the situation the npm ecosystem is in. To lint JS files via ESLint, you need to install the dev dependency, setup it's config, setup the ESLint CLI for the GitHub Actions CI, and find an editor plugin. Then if you also need to lint CSS, you have to install the stylelint dev dependency, setup it's particular config, setup it's CLI for the GitHub Actions CI, and find a separate editor plugin. And so forth for any other syntaxes, although JS and CSS are the big ones.

From the OSS side of things, ESLint, stylelint, and other lint tools are having to design, build, and maintain separate systems for config, CLI, reporting, etc. again and again. General problems solved by one tool (e.g. sharable configs) might take years to be solved for others. A lot of duplicated and wasted effort.

Configs for the various tools end up spread across multiple files with different systems for enabling rules, ignoring files, etc. The ignore comments are stylistically different between tools. You end up with complicated config to get CSS in JS to be linted with stylelint, and JS code examples in markdown to be linted with ESLint, or markdown/CSS/JS in JSDoc fenced code examples to get linted by the right tools. Deno could holistically lint all these mixed syntax contexts. Lint tool CLI's put out slightly different looking reports, and take different args that need to be learned and remembered. Some editors don't have a stylelint extension at all, so you're just shit out of luck in that case and can only use the CLI.

It would be amazing if the Deno CLI and language server could allow linting plugins for any of the Deno formattable/language server languages to be installed. Only one Deno editor extension using the Deno language server needs to be installed. Only one Deno lint CLI needs to be learned, with no installation needed since it's out of the box. Setup one lint config file for all syntaxes. Setup GitHub actions CI once, and then Deno lint plugins and config can be chopped and changed easily at will.

An amazing amount of teams don't do any linting at all, because it's all too fiddly. Or they lint some syntaxes, but not others (e.g. people bother with ESLint but neglect CSS). Deno has an opportunity to get this right from the start, or at least provide a super fast common Rust infrastructure and unified config system for the community to work with and publish third party plugins.

What rule set would you propose for linting Markdown and JSON/JSONC?

For example, linting certain JSON files according JSON schemas.

@bartlomieju
Copy link
Member

@jaydenseric I understand your sentiment and indeed linting codeblocks in Markdown files could be done from our point of view (similar to how deno fmt can format codeblocks in .md files).

I'm not so sure about validating JSON files against JSON schemas - best to open a separate issue about it.

As for CSS - this would be a huge undertaking and we currently don't have any infrastructure to do that, plus Deno doesn't recognise CSS files in anyway. Once support for standardised CSS modules lands we could reevaluate.

We do have a plan to make deno lint more configurable via a config file, but I must warn you, we do not want to go ESLint's way where every rule has several settings that can be tweaked. Plugin support is nascent at the moment, but the same rule would apply regarding configuration of rules.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 8, 2021

For example, linting certain JSON files according JSON schemas.

How do you propose to associate a specific JSON file with a JSON schema?

@bartlomieju
Copy link
Member

For example, linting certain JSON files according JSON schemas.

How do you propose to associate a specific JSON file with a JSON schema?

https://json-schema.org/learn/getting-started-step-by-step - using $schema and $id fields. Though in denoland/deno#11885 @dsherret suggest we shouldn't go with this solution for Deno's config file.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 8, 2021

Yeah, I know about that. My point is "how deep is the rabbit hole"... editors already solve this problem. For example, vscode allows registration of file patterns to schemas and enforces the JSON schema. This is how we support format enforcement and suggestions in import maps in vscode_deno.

It requires lots of configuration though, and is not something well suited for the Deno CLI. Editors and their clients solve it better.

@jaydenseric
Copy link
Author

jaydenseric commented Sep 8, 2021

Deno could show lint errors/warnings for import map JSON files, and Deno config files (whatever form they end up taking). For other third-party tool specific JSON files, schemas could be associated to specific files via user Deno lint config if deno ships a configurable JSON schema rule out of the box, or just by a third party Deno lint plugin. For example, a user could install a third-party Deno lint plugin for linting package.json fields.

I'm not in urgent need of JSON schema linting, it's just a random example off the top of my head of JSON file linting (beyond basic JSON syntax validation) users might want. It's pretty useful when editing VS Code settings files:

Screen Shot 2021-09-08 at 3 21 31 pm

I can't really live without CSS linting, so for now I've hacked together a Deno based CLI that can validate all .css files in a project using the online W3C CSS validator:

Screen Shot 2021-09-04 at 10 39 52 am

It was actually very fun and elegant to code (< 100 lines of TS), since Deno has such great std APIs for ANSI formatting, expanding file globs, etc. It's not a great long-term solution though:

  • Requires an internet connection to use.
  • Speed is bottlenecked by network POST requests.
  • The W3C CSS validator is a free, un-versioned service that could change or cut you off for exceeding request limits at any moment (hasn't happened to me so far though).
  • There is no in-editor linting UI via the Deno language server. Panic provides a CSS validator extension for the Nova editor that also uses a W3C online service, but it has problems and validates differently to my CLI code that is used in CI. I'm not motivated to publish extensions for different editors that only integrate this specific CLI.

@jthegedus
Copy link

jthegedus commented Sep 8, 2021

So you expect deno_lint to lint Markdown and JSON/JSONC files?

Well if there are opinions about how those files should be format then I expect there are pitfalls for writing them that can be linted?

How that looks, I do not know.

At a minimum, updating the documentation for each of the commands to list the file types they apply to by default would be helpful. (this would satisfy the catalyst for me commenting here)


My point is "how deep is the rabbit hole"... editors already solve this problem. For example, vscode allows registration of file patterns to schemas and enforces the JSON schema. This is how we support format enforcement and suggestions in import maps in vscode_deno.

Support for linting a file format that has a spec (eg JSON) and autocomplete of specific implementations of that spec are different IMO. EG: a .json file with a // comment is invalid and a potential rule for a linter to check.


On the further discussions, these are my thoughts & perspective:

Node.js expanded from just a server runtime for JS to being the toolchain for most JS development, both frontend and backend. From my perspective Deno has a clear goal to reduce the fragmentation of the current (JS) ecosystem by shipping a CLI with batteries included (lint, fmt, bundle, compile), so evolving to support format and linting of adjacent tooling warrants discussion. Whether or not it is a good idea will come from discussion.

Using Deno gives me significant vibes to Rome Tools in that:

Rome is a linter, compiler, bundler, and more for JavaScript, TypeScript, JSON, HTML, Markdown, and CSS.

It currently supports Parse, Format & Lint of JS, TS, JSX, JSON, RJSON, HTML, CSS, Markdown.

Additionally, eslint, which is the tool with which feature parity is measured has plugins for css, html etc, so it is certainly possible to support linting with those rulesets. Again, whether or not it is a good idea is another question of which the answer comes from discussion and prototyping.

@jthegedus
Copy link

Futhermore, would deno bundle ever support bundling HTML, CSS & JS/TS, probably not, that doesn't make much sense. But, should the default formatter of the Deno ecosystem support formatting and linting these auxiliary file types, perhaps.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 8, 2021

Let's define a few things here, so we are clear what we are talking about.

  • diagnostics, warnings or errors provided the something like Deno.
  • syntactic errors, for a given language, diagnostics that are errors that usually indicate the code/file is not usable
  • semantic errors, the code is the correct syntax bit is still invalid because it expresses the "wrong" information.
  • formatting, conforming the text of a file to a given set of rules without changing the syntax or semantics of the code.
  • linting, while not precisely defined, any sort of stylistic warnings you would want to convey that are semantic or syntactic in nature. The code works perfectly fine, it just can be more aligned to a chosen set of optional rules.

We already provide syntactic diagnostics for import maps and tsconfig files from the CLI, and will provide them for the config file as we add features. Providing syntactic diagnostics for JSON files in an editor is best left out of the scope of the Deno language server in my opinion. Providing better semantic diagnostics is something we should consider and do IMO. The vscode_deno extension already provides some semantic/syntactic diagnostics for import maps via the ability to map files to JSON Schemas. I prefer that approach as it saves from bloating Deno CLI with a JSON Schema validator. Adding on more semantical errors, like resolving specifiers in the map, is the domain of Deno language server though.

There is no "linting" of JSON files above that of formatting, which is already provided.

Syntactic diagnostics for JSON, in the base sense of "is it valid JSON" would come into scope when JSON modules are supported by Deno. Enforcement of schemas though is not something IMO we should get into with Deno built into the runtime. It really is a concern of user code to enforce such semantics as they see fit.

Applying syntactic diagnostics to code blocks might make sense, but as mentioned that isn't the "Also lint CSS" issue. We have experimented with this with deno test and that continues to feel like the right way to expose this. Also we have found that applying semantic or linting diagnostics though would likely be a problem, because these are often code snippets and not fully expressing everything, leasing to lots of false positives to the point of being unusable.

Outside of code blocks and formatting, syntactic diagnostics generally don't exist for markdown and linting rules seems really like something that Deno should get involved with.

As discussed CSS semantics and syntactics might be in scope when and if CSS imports become part of JavaScript. Linting is still a complicated thing that could only be considered well beyond that point.

@jthegedus
Copy link

I think we agree in the low value that a JSON lint would provide when a formatter would already bubble some of the information to the user.

Having said that, the ruleset is rather small as shown in eslint-plugin-json and would surface the issues to those who expect to find them in a Linter. The desire for this is shown with eslint-plugin-json having 250k downloads/week on npmjs (a crude metric). Similar metrics exist for the HTML and CSS eslint plugins.

Mapping to JSON schemas was not my goal of raising JSON, I agree that the featureset of JSON schemas is another issue entirely and personally think it should live at the extension layer as mentioned.

JSON was merely an example of the three (JSON, HTML, CSS) file formats that Rome has committed to delivering linting for which Deno has yet to commit specific support for.

I landed here because I wanted to know what Deno could lint. I had an idea of what it could format and was surprised they differed. I see other tools supporting both formatting and linting in unison for file types, even if there is overlap between lint & format, or the ruleset is trivial, or the ruleset a stricter subset of an already supported ruleset.

I will let @jaydenseric resume this conversation specifically about CSS.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 8, 2021

JSON was merely an example of the three (JSON, HTML, CSS) file formats that Rome has committed to delivering linting for which Deno has yet to commit specific support for.

Deno and Rome's design goals are quite a bit different.

The desire for this is shown with eslint-plugin-json having 250k downloads/week on npmjs (a crude metric).

Or someone has included it in an opionsted stack and 250k people a week are downloading something they aren't aware of and don't consciously use, in the never ending bloat that is the npm black hole?

@jthegedus
Copy link

Or someone has included it in an opionsted stack and 250k people a week are downloading something they aren't aware of and don't consciously use, in the never ending bloat that is the npm black hole?

Yes, I stated this was a crude measurement.

Deno and Rome's design goals are quite a bit different.

Yes, Rome will utilise Node and it's package ecosystem and so it's goals are different to Deno. The tools undoubtedly have overlap in the "batteries-included" aspects (linting, formatting etc) of which I was trying to discuss. If the minds behind Rome see a purpose in including the aforementioned, then I think that warrants sincere consideration.

@kitsonk
Copy link
Contributor

kitsonk commented Sep 9, 2021

Rome is development tooling to develop web applications. Deno is a web platform runtime. They are very distinct things.

@jthegedus
Copy link

I understand that, I feel you are specifically ignoring the actual points I am trying to raise.

@dsherret
Copy link
Member

dsherret commented Sep 9, 2021

This conversation on JSON and markdown (differences between deno fmt) feels off topic. Maybe there should be a separate issue?

@RobertAKARobin
Copy link

Linting CSS is really a solved problem, thanks to Stylelint. Instead of Deno reinventing the wheel I would prefer Deno just introduce a way to run Stylelint.

@jrson83
Copy link

jrson83 commented Nov 9, 2022

Linting CSS is really a solved problem, thanks to Stylelint. Instead of Deno reinventing the wheel I would prefer Deno just introduce a way to run Stylelint.

Yes, you cannot use the vscode stylelint extension with deno and for me it is also not working when installed stylelint globally with npm. It is really annoying. If you don't want to install stylelint with npm inside a deno project, the only way using stylelint is to install it outside the project folder and set a relative path in the extension stylelint path config.

@RobertAKARobin
Copy link

Sort-of related to #25, which concerns Eslint.

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

No branches or pull requests

7 participants