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

Make a few exceptions to the standard rules #2

Closed
zeke opened this issue Aug 8, 2016 · 4 comments
Closed

Make a few exceptions to the standard rules #2

zeke opened this issue Aug 8, 2016 · 4 comments

Comments

@zeke
Copy link
Owner

zeke commented Aug 8, 2016

The standard linter has a few rules that feel restrictive when applied to javascript blocks in markdown files. While standardizing electron's docs, these are the rules I came across that I think we could do without:

Disallow Undeclared Variables (no-undef)

// `win` is not declared in this block
win.close()

Disallow Unused Variables (no-unused-vars)

// `BrowserWindow` is declared but not used
const {BrowserWindow} = require('electron')

Disallow Unnecessary Nested Blocks (no-lone-blocks, no-labels)

Example electron/electron#6748

// this object is just dangling in space
{
  marginsType: 0,
  printBackground: false
}

How?

I think the way to accomplish this is by forking eslint-config-standard, then using standard-engine to load it. We can use the numerous standard forks for reference.

@ferros, @Flet: If we only wish to deviate very slightly from standard's defaults, is there a way to codify the differences in a package.json stanza? Or is standard-engine with a custom eslint package the way to go?

cc @kevinsawicki @jlord @zcbenz

@MarshallOfSound
Copy link
Collaborator

@zeke So we don't have to fork things crazily. Couldn't we "hack" in eslint disable comments to the JS blocks we are parsing.

I.e. We have a code block block

const disabledRules = ['no-undef', 'no-unused-vars', 'no-lone-blocks', 'no-labels'];
let block = getCodeBlockThing();
block = `
/* eslint-disable ${disabledRules.join(', ')} */
` + block;
standard.lintText(block)

This is untested, just an idea 😄

@feross
Copy link

feross commented Aug 11, 2016

I didn't see this until now because you misspelled my name, @zeke 😜

Disabling these rules makes standard-readme way more useful -- good call! They're the reason I haven't adopted it on my projects yet.

@MarshallOfSound's suggesting is the best solution IMO. It's easiest and easy to maintain -- will have the exact same rules, dependency versions, etc. as the latest stable standard.


Another option, but less ideal, is to extend standard to layer your rule changes on top. No need to fork eslint-config-standard to do this.

You can make a eslintrc.json file like this one:

{
  "extends": ["standard", "standard-jsx"],
  "rules": {
    "no-undef": 0,
    "no-unused-vars": 0
  }
}

Install all the standard dependencies. Then use the eslintrc.json with standard-engine.

One big downside is now it's your responsibility to ensure that the package versions are kept up-to-date and match what's in standard. You can try just always using the latest versions with * and it will probably work fine. But there are some situations when ESLint has a bug where we hold back the semver range to an older version until it's fixed. So you lose that with this approach.

@zeke
Copy link
Owner Author

zeke commented Aug 15, 2016

I didn't see this until now because you misspelled my name

Doh! I do know how to spell it. I blame my fingers. 🖖

@zeke zeke closed this as completed in #3 Aug 15, 2016
@zeke
Copy link
Owner Author

zeke commented Aug 16, 2016

#3 and #6 landed in 1.2.0 🚀

Thanks for the help, @MarshallOfSound and @feross!

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

3 participants