-
Notifications
You must be signed in to change notification settings - Fork 128
Add eslint rules to match Firefox #2365
Comments
As close as I can figure, here's the mozilla-central .eslintrc.js config file: "use strict";
module.exports = {
// When adding items to this file please check for effects on sub-directories.
"plugins": [
"mozilla"
],
"rules": {
"mozilla/avoid-removeChild": "error",
"mozilla/import-globals": "warn",
"mozilla/no-import-into-var-and-global": "error",
"mozilla/no-useless-parameters": "error",
"mozilla/no-useless-removeEventListener": "error",
"mozilla/use-default-preference-values": "error",
"mozilla/use-ownerGlobal": "error",
// No (!foo in bar) or (!object instanceof Class)
"no-unsafe-negation": "error",
},
"env": {
"es6": true
},
"parserOptions": {
"ecmaVersion": 8,
},
}; And digging further, here's the main source for the eslint-plugin-mozilla plugin: "use strict";
/**
* Based on npm coding standards at https://docs.npmjs.com/misc/coding-style.
*
* The places we differ from the npm coding style:
* - Commas should be at the end of a line.
* - Always use semicolons.
* - Functions should not have whitespace before params.
*/
module.exports = {
"env": {
"node": true
},
"rules": {
"brace-style": ["error", "1tbs"],
"camelcase": "error",
"comma-dangle": ["error", "never"],
"comma-spacing": "error",
"comma-style": ["error", "last"],
"curly": ["error", "multi-line"],
"handle-callback-err": ["error", "er"],
"indent": ["error", 2, {"SwitchCase": 1}],
"max-len": ["error", 80, 2],
"no-multiple-empty-lines": ["error", {"max": 1}],
"no-undef": "error",
"no-undef-init": "error",
"no-unexpected-multiline": "error",
"object-curly-spacing": "off",
"one-var": ["error", "never"],
"operator-linebreak": ["error", "after"],
"semi": ["error", "always"],
"space-before-blocks": "error",
"space-before-function-paren": ["error", "never"],
"keyword-spacing": "error",
"strict": ["error", "global"],
},
// Globals accessible within node modules.
"globals": {
"DTRACE_HTTP_CLIENT_REQUEST": true,
"DTRACE_HTTP_CLIENT_RESPONSE": true,
"DTRACE_HTTP_SERVER_REQUEST": true,
"DTRACE_HTTP_SERVER_RESPONSE": true,
"DTRACE_NET_SERVER_CONNECTION": true,
"DTRACE_NET_STREAM_END": true,
"Intl": true,
},
}; It's mildly unfortunate that the eslint-plugin-mozilla doesn't export a "recommended" config so we could easily just do something like this, but hey, what can you do... extends:
- eslint:recommended
- plugin:mozilla/recommended UPDATE: Filed plugin:mozilla/recommended request in bugzilla as 1347712. |
@ianb - we have two options:
If we do 1, then I'd say, at a minimum, we want to also add a few more rules to pageshot's configuration, especially no-undef (there's various helpers for this). |
I'd like to just integrate the entire of the eslint config. My only concern is it tends to require a big diff that conflicts with other changes, so whoever does it should try to get it done quickly once they start. |
In other interesting news, I can't get the latest eslint-plugin-mozilla versions to work at all. I created https://github.com/pdehaan/eslint-mozilla yesterday and tried using latest versions and Node 6/7 and it just fails with some error about not being able to find eslint-plugin-mozilla (even though eslint and eslint-plugin-mozilla are both installed locally -- see below). Interestingly, eslint-plugin-mozilla works in the mozilla/activity-stream repo, but it's an older version (eslint-plugin-mozilla@0.0.3). As soon as I update to ➜ activity-streams git:(master) ✗ $(npm bin)/eslint . --ext=.js,.jsx
It looks like npm has the following published versions: $ npm info eslint-plugin-mozilla versions
[ '0.0.3', '0.2.1', '0.2.3', '0.2.5', '0.2.28', '0.2.29' ] Both /cc @Standard8 |
Ok, I'll bite, we've about 3 projects wanting to use something similar now, so it is time to come up with a better solution. @pdehaan I thought 0.2.29 should have been working a bit better, but its possible that I missed something. I've got some ideas for how to make this work properly, so I'll start on those. |
@Standard8: One small question. I seem to be stuck in eslint-plugin-mozilla/lib/environments/places-overlay.js: var modules = require(path.join(root,
"tools", "lint", "eslint", "modules.json")); This seems to be where my build is failing. I was hacking in my node_modules directly and did some garbage like this: var root = helpers.getRootDir(module.filename);
var tmp = path.join(root, "tools", "lint", "eslint", "modules.json");
console.log(tmp);
// var modules = require(tmp);
var modules = []; We're trying to do a dynamic I'm guessing that's why this is crashing hard, because I don't have a "tools/lint/eslint/modules.json" file in my Page Shot repo tree (but it looks like it's a path in the m-c repo: http://searchfox.org/mozilla-central/source/tools/lint/eslint/modules.json). UPDATE: Submitted 1347709 in Bugzilla, like a big boy! |
I noticed some eslint failures on import. Because of that, and just for general conformity, we should use the same rules as the Firefox tree.
@Standard8: for you?
The text was updated successfully, but these errors were encountered: