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

3.0 feedback #14

Closed
9 tasks done
popomore opened this issue Jul 8, 2016 · 54 comments
Closed
9 tasks done

3.0 feedback #14

popomore opened this issue Jul 8, 2016 · 54 comments

Comments

@popomore
Copy link
Member

popomore commented Jul 8, 2016

待讨论规则

  • dot-notation
  • no-magic-numbers
  • no-console
  • valid-jsdoc
  • quote-props
  • prefer-template
  • no-extra-parens
  • max-len
  • no-unused-expressions
@popomore
Copy link
Member Author

popomore commented Jul 8, 2016

Symbol.for('egg#loader')

ret.then(result => {
   callback(null, result);
}).catch(callback);

.for is a syntax error dot-notation

@fengmk2
Copy link
Member

fengmk2 commented Jul 8, 2016

  • no-magic-numbers 要设置为 off
  • no-console 很难避免的
let pkg;
try {
  pkg = require(path.join(root, 'package.json'));
} catch (err) {
  console.error('read package.json error: %s', err.message);
  console.error('[egg-ci] stop create ci yml');
  process.exit(0);
}

@popomore
Copy link
Member Author

popomore commented Jul 8, 2016

Line 37 exceeds the maximum line length of 100 max-len

有点短

@atian25
Copy link
Member

atian25 commented Jul 8, 2016

valid-jsdoc 这个, @returns 和 ``@return` 是否允许同时存在?

用 webstorm 的时候, 默认生成的是 @returns, 但手写一般都是 @return

@popomore
Copy link
Member Author

popomore commented Jul 8, 2016

Unquoted reserved word 'package' used as key quote-props

@atian25
Copy link
Member

atian25 commented Jul 8, 2016

// config/plugin.js
exports.combo = {
  package: '@ali/egg-combo'
};

Unquoted reserved word 'package' used as key quote-props

@atian25
Copy link
Member

atian25 commented Jul 8, 2016

4:25 error Gratuitous parentheses around expression no-extra-parens

会导致下面这种写法提示错误

module.exports = app => (
   class Test extends app.Proxy {
   }
);

因为下面这种写法也会提示错误, 我会倾向于上面那种写法.

module.exports = app => {
   return class Test extends app.Proxy {
   }
};

@atian25
Copy link
Member

atian25 commented Jul 8, 2016

@fengmk2 在 egg 里面, 应该很少用 console 吧? logger 呢?

@popomore
Copy link
Member Author

popomore commented Jul 8, 2016

quote-props 感觉也可以用掉

@popomore
Copy link
Member Author

popomore commented Jul 8, 2016

用下来感觉 prefer-template 也太强制了

@popomore
Copy link
Member Author

popomore commented Jul 8, 2016

已经整理到上面了

@atian25
Copy link
Member

atian25 commented Jul 8, 2016

嗯,template 那个不够智能,很短的也要求用。

@atian25
Copy link
Member

atian25 commented Jul 8, 2016

配套的 eslint 用3.x ?

@fengmk2
Copy link
Member

fengmk2 commented Jul 8, 2016

generator-star-spacing 有问题,有没空格都是 ok 的,可不写就不写。

function* generator() 这样 ok,但是 function*() 应该支持不加空格。

@fengmk2
Copy link
Member

fengmk2 commented Jul 8, 2016

prefer-template 设置为 off 吧

@fengmk2
Copy link
Member

fengmk2 commented Jul 8, 2016

像正则很可能就超过 max-len 了。。。

        .should.match(/\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2},\d{3} INFO \d+ \[-\/-\/127.0.0.1\/-\/0ms GET \/\] info foo\n/);

@atian25
Copy link
Member

atian25 commented Jul 9, 2016

@fengmk2 我之前的实践是:

  • 提供一个 egg/test 的规则集, 在测试脚本里面适当的放松规则.
  • 正式代码里面遇到 max-len 这种少数场景, 单独使用 // eslint-disable-line max-len

@popomore
Copy link
Member Author

popomore commented Jul 9, 2016

设 132 吧,或者不要设置了,对我们编码其实影响不大

http://programmers.stackexchange.com/questions/148677/why-is-80-characters-the-standard-limit-for-code-width

@popomore
Copy link
Member Author

popomore commented Jul 9, 2016

dot-notation allowKeywords 设为 true

no-magic-numbers 关掉

quote-props 关闭 keywork

@popomore
Copy link
Member Author

popomore commented Jul 9, 2016

no-unused-expressions 会导致 app1.use.should.be.a.Function; 挂掉

popomore added a commit that referenced this issue Jul 9, 2016
popomore added a commit that referenced this issue Jul 9, 2016
@atian25
Copy link
Member

atian25 commented Jul 9, 2016

prefer-template,no-extra-parens 也关掉吧。。

no-console 我感觉是使用场景少,可以开着?

valid-jsdoc 这个只是想看看有没有同时兼容两种 return 的配置。关的话会不会使其他文档注释不规范? 感觉可以开着。

@popomore
Copy link
Member Author

这个大家再看看,我一起改了

@atian25
Copy link
Member

atian25 commented Jul 10, 2016

再提供一个 egg/lib/test 的规则集,适当放松一些规则,如 func-name之类的

@popomore
Copy link
Member Author

感觉没必要吧,都是 js 为啥要区分

@atian25
Copy link
Member

atian25 commented Jul 10, 2016

也行吧,我之前的实践是,测试里面的函数无需有名字,还有关闭maxlen之类的。

@popomore
Copy link
Member Author

写成 arrow function 呗

popomore added a commit that referenced this issue Jul 10, 2016
@atian25
Copy link
Member

atian25 commented Jul 12, 2016

3.0.3 建议:

module.export = app => (
  /**
   * 不用 return, 还能写注释
   */
  class Test extends app.Proxy {

  }
);

@dead-horse
Copy link
Member

eslint/espree#140 使用的这个 parser 不支持 async function,我好想不能在外部的 eslintrc 上覆盖掉这个 parser... 换成 babel-eslint 会报错

@dead-horse
Copy link
Member

切换成 babel-eslint 后因为这个报错: eslint/eslint#6274

可以设置 rules 为:

"generator-star-spacing": 0,
"babel/generator-star-spacing": 0,

�我们的 parser 用 babel-eslint ? ava 写测试的时候引入了 async function

@dead-horse
Copy link
Member

sourceType: 'modules',

这一句写错了,正确的应该是 module,但是如果设置成 module 其实是要用 import 和 export,且不写 'use strcit', 所以应该是写成 sourceType: 'script' ?

@dead-horse
Copy link
Member

console 还是会用到,如果要开着的话我要用 const logger = console 来避免,感觉有点蛋疼

@atian25
Copy link
Member

atian25 commented Jul 12, 2016

sourceType 不应该是 module

@atian25
Copy link
Member

atian25 commented Jul 12, 2016

console 关+1 。 我现在是前端代码那里单独配置,不允许 log ,可以 error 和 warn

@dead-horse
Copy link
Member

在项目里面升级了一下,要修改的地方为:

module.exports = {
  extends: 'eslint-config-egg',
  parser: 'babel-eslint',
  rules: {
    'generator-star-spacing': 0,
    'babel/generator-star-spacing': 0,
  },
};

前端使用了 react 和 import / export,所以再覆盖了一份:

module.exports = {
  parserOptions: {
    sourceType: 'module',  // import & export 的支持,这个模式下的文件不能写 use strict
  },
  plugins: [
    'react',
  ],
  rules: {
    'react/jsx-uses-vars': 1,  // 在 jsx 里面引用的变量默认检测不到
    'no-extra-parens': 0,  // 经常会有写法是 return (<div></div>) 这样的,这个暂时打开了
  },
};

感觉可以将非前端的那部分配置挪到 eslint-config-egg 里面去,然后前端相关的在文档里面写明。

@popomore
Copy link
Member Author

恩,改下呗

@popomore
Copy link
Member Author

我建议 eslint-config-egg 不要加 node 不支持的特性,这个本身是用于 node 的代码规范,如果要公用那就覆盖他好了。

@popomore
Copy link
Member Author

我看文档 script 是默认的

@atian25
Copy link
Member

atian25 commented Jul 12, 2016

react相关的是不是独立一个库? 印象中应用那里可以 extend多个的。

@atian25
Copy link
Member

atian25 commented Jul 12, 2016

或者还在这个里面,但只是 export 出来,不默认使用。

@popomore
Copy link
Member Author

是遍历每一层目录然后合并的,在前端目录放个配置就好了,而且前端框架也比较多,很多人会直接用 airbnb 之类的。

@atian25
Copy link
Member

atian25 commented Jul 13, 2016

我的建议是: 如果前端使用的规则比较多, 可以做个小合集.

egg 可以 export 多个, 如 egg 和 egg/lib/react, 然后 views 里面继承多个.

# .eslintrc.yml
---
extends: egg
rules:
  no-console: 'error'

# app/views/.eslintrc.yml
---
extends: 
  - ../../.eslintrc.yml
  - egg/lib/react

rules:
  no-console: [ "error", { allow: ["info", "warn", "error"] } ]

# eslint-config-egg/lib/react
---
parserOptions:
  sourceType: "module"
  ecmaFeatures:
    experimentalObjectRestSpread: true

@atian25
Copy link
Member

atian25 commented Jul 13, 2016

@dead-horse 在那个 PR 里面我改了 sourceTypeno-console

@dead-horse
Copy link
Member

前端的不用默认配置,export 一份或者在文档里面说明(感觉文档说明比较合理,否则要引入额外的依赖 eslint-pluin-react 这些)。

不过是否把 parser 默认替换成 babel-eslint 呢?这样默认是支持 es7 语法的,依赖也不大,不过大部分情况下确实也可以让用到 babel 的人自行引入,但是需要文档中注明 generator-star-spacing 的坑..

@dead-horse
Copy link
Member

no-extra-parens 感觉可以关掉,很多时候会加括号来帮助优先级判断,例如:

if (a === 1 || (b === 2 && c === 3) ) {
  // do something
}

@atian25
Copy link
Member

atian25 commented Jul 13, 2016

@dead-horse 我在那个 PR 里面改为 no-extra-parens: [ 'error', 'functions' ] 了, 只限制函数

http://eslint.org/docs/rules/no-extra-parens

@popomore
Copy link
Member Author

@dead-horse 如果是多余的话说明优先级是对的,&& 高于 ||

@geekdada
Copy link
Member

no-extra-parens: [ 'error', 'functions' ] +1

@dead-horse
Copy link
Member

@popomore 我知道这个优先级是对的,但是对阅读代码来说加一个括号更优

@atian25
Copy link
Member

atian25 commented Jul 13, 2016

@popomore 我个人更喜欢多余的方式, 避免去猜, 就像以前的 ++i++ 之类的

@fengmk2
Copy link
Member

fengmk2 commented Jul 13, 2016

@popomore 我个人更喜欢多余的方式, 避免去猜, 就像以前的 ++i++ 之类的

+1

@popomore
Copy link
Member Author

@atian25
Copy link
Member

atian25 commented Jul 13, 2016

@dead-horse parser 那个你新开个 issue ?

@dead-horse
Copy link
Member

我单独开个 MR,补一下 README 好了,先不将那些引入到 egg 的配置

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

5 participants