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

Making standard easier to use #1356

Open
feross opened this issue Aug 13, 2019 · 15 comments
Open

Making standard easier to use #1356

feross opened this issue Aug 13, 2019 · 15 comments
Labels

Comments

@feross
Copy link
Member

feross commented Aug 13, 2019

The vision of standard is to save you time in three ways:

  1. No configuration
  2. Automatically format code
  3. Catch style issues & programmer errors early

I think we do a really good job on (3), enforcing style and catching errors. We do a pretty good job on (2) as well, thanks to ESLint's --fix functionality.

But we can do way better at Number 1.

We're pretty far away from the vision of "just do this and you're done" for lots of real-world projects.

$ npm install standard
$ standard

In fact our readme promises "no configuration" and then goes on to do a bit too much explanation about packages to install and flags to pass to get standard to work for your particular codebase. And it's not working so well right now.

Here are some very rough ideas that that might improve the situation. Feedback very much welcome!


  • Include babel-eslint by default

    • One less package for Babel users to install
    • If the user is already using Babel, then their .babelrc is recognized and used automatically
    • No noticeable difference for non-Babel users (Note: need to verify if performance is same)
  • Remove --verbose flag and just always include the rule name in the error message

    • I think users almost always want to know the rule name they can google it to learn more

ESList exposes features that we decided to remove/hide for simplicity. But then users sometimes need these features, so we created additional packages to install as an escape hatch to get those features back. I'm not sure that this is actually simpler than just adding a command line flag for these features. So here's a few ideas:

  • Build in snazzy so users can add do standard --report=pretty and get pretty output
  • Build in standard-json so users can do standard --report=json to get JSON output
  • Build in standard-tap so users can do standard --report=tap to get TAP output

ESLint already supports these internally so it feels kind of strange to have to maintain these packages which just parse output and convert it back.

Note: these flags are not allowing arbitrary rules to be added or changed, so they're not the kind of "configuration" that we want to avoid. These are just command line flags that help users accomplish their goals. I can't see teammates bikeshedding about whether to use --pretty or not, and especially not for --json.


  • Allow all non-confusing browser globals Consider allowing use of browser globals #1330

    • Eliminate the need to use a window. prefix or a /* global Blah */ comment
    • Code that uses window. is not usable in a service worker or in Node.js
    • globalThis is not pretty and not widely available yet anyway.
    • Confusing browser variables like top and self will still be treated as undefined by default
  • Build in prettier Adopt prettier for consistent formatting #811

    • Users who want deterministic formatting on every save can just do standard --format.
    • Users who want minimal changes to their code, can continue using standard --fix.
    • Eliminates the need for additional wrapper packages and users to install multiple tools.
  • Lint JavaScript in Markdown files and HTML files by default Lint all JavaScript in markdown files by default #1355

    • All my code examples in README.md files and docs/ folders should be standard, but I never remember to install standard-markdown. We can build this in by default.

The TypeScript experience is completely broken right now. Typescript users need to install an ESLint plugin and configure some rules in order to get standard working. The problem is that you can't configure rules in standard. Indeed, that's the point. So they have to switch to standardx, another escape hatch that we built to handle cases that we don't handle correctly in standard itself.

Users also have to do this to get set up:

npm install @typescript-eslint/parser @typescript-eslint/eslint-plugin --save-dev

Followed by

standard --parser @typescript-eslint/parser --plugin @typescript-eslint/eslint-plugin *.ts

Ugh. This looks like configuration to me. Here's a possible solution:

  • Support Typescript out of the box TypeScript linting works only partially #1283
    • Detect .ts files and apply the necessary minimal rule changes so everything works.
    • Bundle the necessary parser and plugin so it just works.
    • Then typescript users can just do standard

If we support parsing Typescript and we default to the babel-eslint parser, then we can also:

  • Remove the --plugin and --parser flags.
    • No more need for users to understand this whole aspect of ESLint.
    • If you want to use plugins, different parsers, etc. then you should just use ESLint directly.

(An alternative is to just drop support for Typescript entirely if we can't do a good job of it)


Here's an idea that might be going too far, but if it worked would be really cool.

  • Support extensions Extensions proposal (not about making Standard "configurable") #703
    • Users can do e.g. standard --react or standard --vue and get additional rules for their specific framework. There can be a very small list of possible choices, maintained as shareable configs in the @standard org.
    • We can provide helpful warnings and bug catching rules to users of e.g. React, etc. that we currently can't do because these rules would apply to everyone.
    • There is already a concept of an env or an ESLint environment with the --env flag. Maybe this can piggyback on that concept?
    • So users could do e.g standard --env react --env mocha if they want additional rules for React and globals defined for Mocha tests, as one hypothetical example.

If we make all the changes I've proposed here, we'd go from this help page:

Flags:
        --fix       Automatically fix problems
    -v, --verbose   Show rule names for errors (to ignore specific rules)
        --version   Show current version
    -h, --help      Show usage information

Flags (advanced):
        --stdin     Read file text from stdin
        --global    Declare global variable
        --plugin    Use custom eslint plugin
        --env       Use custom eslint environment
        --parser    Use custom js parser (e.g. babel-eslint)

to this:

Flags:
        --fix       Automatically fix problems
        --format    Aggressively format code for consistency
        --version   Show current version
    -h, --help      Show usage information

Flags (advanced):
        --stdin     Read file text from stdin
        --report    Use custom reporter (choose from: pretty, json, tap)
        --global    Declare global variable
        --env       Use custom environment (choose from: react, vue, mocha, etc.)

If I had to summarize the main idea here I'd say it feels like we've let a lot of the complexity of ESLint leak through, and we can do better. Lots of the details of my proposed solutions might not be the best, but I hope this will re-energize us and get us thinking about solutions.

Eager for feedback!

@Pandawan
Copy link

Pandawan commented Aug 14, 2019

(An alternative is to just drop support for Typescript entirely if we can't do a good job of it)

Or, if you decided not to add TypeScript support by default, you might want to consider supporting it as a plugin like you mentioned for react and vue instead?

Note: This should still be an "alternative," it'd be much better if standard supported TypeScript by default with no extra config/installs.

@feross
Copy link
Member Author

feross commented Aug 14, 2019

Or, if you decided not to add TypeScript support by default, you might want to consider supporting it as a plugin like you mentioned for react and vue instead?

Yeah, that's a good point. It would fit easily into that model since it's just a plugin, parser, and a few rule tweaks.

@mightyiam
Copy link
Contributor

👍 Babel by default.
👍 Including rule name in message by default and removing --verbose.
👍 Pretty output by default.
👍 Built-in JSON and TAP reporters

#1330 seems too large to be yes/no in the context of this issue.

I have a feeling that bundling prettier is not a worthwhile investment. Perhaps making it officially supported and very easy to use along with standard would be wiser. I can imagine a future where ESLint fixes make prettier obsolete. You can say I'm a dreamer. I've never used prettier.
Formatting people's code goes beyond what we do here. If we decide to ever replace prettier with anything else, then the code breakages could be enormous. It's a can of worms, I tell ya ☠️

👍 #1355.

👍 for #703 (I wrote it) but without the need for a CLI flag. Just having the extension package installed should activate it.

@LinusU
Copy link
Member

LinusU commented Aug 14, 2019

I have a feeling that bundling prettier is not a worthwhile investment[...]

Personally, this is one of the things I'm looking forward to the most! I've been in a lot of projects that used to use standard, but has switched to Prettier. It's always a hassle since Prettier is impossible to configure to 100% match standard, so in the end we'll end up adding a prettier config to match as closely as possible, and then either run eslint after (which preserves all the good "catch bug" linting) or just skip standard alltogheter.

If all we needed to do was to add a --format flag to the standard command, that would be amazing and a huge win IMHO 😍

@bcomnes
Copy link
Member

bcomnes commented Aug 14, 2019

How much larger will the install be with the babel stuff?

@ArmorDarks
Copy link

This mostly looks like a nice one!

Include babel-eslint by default

Including babel-eslint doesn't seem right for people that do not use Babel (hello, TypeScript...). Maybe just to allow standard check for current project dependencies, and if it sees babel-eslint automatically enable it, without the need to specify --plugin?

That means that the user still has to install babel plugin, but doesn't need to configure it.

Build in prettier #811

Adopting Prettier seems to be the right decision, as the war between eslint, standard and prettier preferences is getting quite annoying.

Lint JavaScript in Markdown files and HTML files by default

nice, but means even more default dependencies...

Support Typescript out of the box

sounds nice, but isn't that more dependencies for non-TypeScript users? Plugins (extensions) still feels better. Just npm install @standard/typescript and standard automatically should pick it up.

(An alternative is to just drop support for Typescript entirely if we can't do a good job of it)

oh no, that will sound like an end of the standard. I've already mentioned that problem — because of the "closed" philosophy, it's risky for anyone to adopt technology which suddenly can become unusable with newer techs just because the user has no way to adjust it to the pipeline. And by saying no to common nowadays techs, we'll show that tech is definitely not viable.

There can be a very small list of possible choices, maintained as shareable configs in the @standard org

The question is, will that list update frequently enough? If for some reason project will stagnate (as that happened a few times, because, well, we all lazy and rely on core maintainers...), it could quickly lose users. That partially happened because of the TypeScript, or Vue, which were unusable with standard for a very long period of time.

Would allowing extension of the standard with a 3rd-party extension be that bad? Technically, they are not that much configuration. Yeap, they might "shift" defined standards, but I think that ultimately what usually extensions do. Some people suggested to disallow overriding of the current rules and allow the only extension, but there'll be too many times when it won't work. An example — TS plugin — it needs to disable some default standard rules to make it work.

It feels to me that it would be much better if standard would provide the community with means to quickly adapt and extend at times when it needed...

So users could do e.g standard --env react --env mocha if they want additional rules for React and globals defined for Mocha tests, as one hypothetical example.

That concept looks interesting.

It also means that certain features can be invoked ad hoc, in some files, right? For instance, standard can't know that particular .js file is in fact a React file, but by specifying env comment in the beginning of the file that could be denoted and standard would apply some additional rules to that file. Sounds nice.

@feross
Copy link
Member Author

feross commented Aug 15, 2019

How much larger will the install be with the babel stuff?

We'd go from 287 dependencies to 321.

As for the performance of babel-eslint, I'm pleased and quite surprised to say that it actually appears to be faster than the built-in ESLint parser.

I ran time npm run test -- --offline and got 33.350 seconds with babel-eslint vs. 45.248 seconds with the built-in parser.

I say that we ship this for the performance improvement alone!

EDIT: I forgot to clear the cache between runs. The new times are 52.463 seconds with babel-eslint vs. 46.336 seconds with the built-in parser. So babel is actually a bit slower.

@sheerun
Copy link

sheerun commented Aug 29, 2019

@feross Maybe also for ease of development you could move standard-engine to this repository?

@sheerun
Copy link

sheerun commented Aug 29, 2019

Also I woudn't complain about adding other packages in monorepo style:

packages
├── ...
├── eslint-config-standard
└── standard-engine

Why? It's hard to coordinate changes in all repositories. For example to add some change in eslint-config-standard I'd need to make request to it, then update standard-engine, then update standard, very tedious. I'd like to help but the fragmentation of standard repositories makes me meh :)

@Janpot
Copy link

Janpot commented Sep 9, 2019

I guess it would be possible to add the --format option without necessarily bundling prettier? I mean, the decision of adding this option shouldn't depend on the decision of bundling prettier. I'd be happy to have built-in prettier, but it would already help me a lot if --fix didn't clash with prettier so much.

@ghost
Copy link

ghost commented Mar 4, 2020

Could we have globalThis enabled by default without the need to set it in package.json as

"standard": {
  "globals": [ "globalThis" ]
}

All modern browsers support it https://caniuse.com/#search=globalThis

@LinusU
Copy link
Member

LinusU commented Mar 5, 2020

@catalin-luntraru that seems like a good move, would you mind opening a separate issue for that, and maybe a PR as well?

@zhangyuang
Copy link

@feross same issue, i want to. use standard with vue, but when i run command like standard --parser babel-eslint --plugin vue **/*.vue,vue files still prompt error

@ChildishGiant
Copy link

Could we have globalThis enabled by default without the need to set it in package.json as

Supporting --env browser would be a much better fix. Each file can currently have /* eslint-env browser */ tacked on at the top but that's not very elegant

@mightyiam
Copy link
Contributor

#1872.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

12 participants