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

[RFC] egg-bin check #2088

Closed
4 of 5 tasks
atian25 opened this issue Feb 7, 2018 · 9 comments
Closed
4 of 5 tasks

[RFC] egg-bin check #2088

atian25 opened this issue Feb 7, 2018 · 9 comments

Comments

@atian25
Copy link
Member

atian25 commented Feb 7, 2018

背景

// config/config.default.js
exports.view = {};
module.exports = appInfo => {
  const config = {};
  config.keys = '123456';
  return config;
};

这种低级错误,在日常解答中出现过很多次,甚至集团内部都有,有时候真会让我有点怀疑人生。
累计到昨天光我自己就解答了 18 次了,不自觉的总会带有一点怒气的去回复,但这样怼人其实真的不好。

如何能帮助开发者避免低级错误,又能保持好维护者的答疑心情呢?

思路

需要引入静态代码检查,但能遇到这类问题的大部分是初级开发者,很少会使用 eslint 。

一种思路是:

  • 通过 eslint 自定义规则来检查代码,封装了独立的包,如 eslint-config-egg-badsmell(最终使用 eslint-plugin-eggache)
  • 提供 egg-bin check 方法来检查,维护者怀疑是低级错误时,让开发者先执行命令,避免无谓的沟通。
  • 可以将内部的 stc 库一并考虑。 cc @jtyjty99999

@天意: stc是用来查 “正确但是有风险的代码” 这应该是属于lint了 属于egg本身的lint

规则库

no-override-exports

  • 检查 config/config.*.js,禁止覆盖 exports
// config/config.default.js
exports.view = {};
module.exports = appInfo => {
  const config = {};
  config.keys = '123456';
  return config;
};

plugin-property

  • 场景:混淆了插件的开启和配置。
  • 检查 config/plugin.*.js,禁止多余的 key,目前仅支持 enable/package/path/env
// config/plugin.js
exports.view = {
  enable: true,
  package: 'egg-view',
  // should config at config.default.js
  dir: 'xxxx',
}

实现

@solarhell
Copy link
Contributor

😂 看你回复都很无奈了

@atian25
Copy link
Member Author

atian25 commented Feb 7, 2018

哈哈

➜  egg-showcase git:(repl) ✗ n eslint . --fix

/Users/tz/Workspaces/coding/github.com/atian25/egg-showcase/config/config.default.js
  25:1  error  Don't use `exports` after `module.exports`  eggache/no-override-exports

✖ 1 problem (1 error, 0 warnings)

@ngot
Copy link
Member

ngot commented Feb 8, 2018

还有个提议,可以根据开发着的开发机的语言配置,来设置提示语言为英文还是中文。我遇到的有相当一部分开发者,看到英文的提示是直接不读的,然后截图丢过来,让你解决。这种非常无奈,英文的写的再直白,开发者不看,也没有办法。

@atian25
Copy link
Member Author

atian25 commented Feb 8, 2018

https://github.com/eggjs/eslint-plugin-eggache

已经实现了 2 个规则,下一步集成到 eslint-config-eggegg-bin check

@ngot 你这个在做 check 时再看看,我的思路是开发者一般都不会跑这个的,当他们提问的时候,让他们执行完贴日志,我们再跟进,这样省一点口水,如果这样的话,i18n 不是很必要。

另外,check 里面是可以考虑用 https://eslint.org/docs/developer-guide/working-with-custom-formatters 来自定义 eslint 的错误输出。

@fengmk2
Copy link
Member

fengmk2 commented Feb 11, 2018

@ngot 中文的话,有些人的终端还没法显示。。。

是否可以检查一下 module.exports[key] 和 exports[key] 是否相同?如果不是,直接在 dev 阶段报错,prod 阶段警告?

@atian25
Copy link
Member Author

atian25 commented Feb 11, 2018

中文的话,有些人的终端还没法显示。。。

嗯,也是考虑到这个...

是否可以检查一下 module.exports[key] 和 exports[key] 是否相同?如果不是,直接在 dev 阶段报错,prod 阶段警告?

这个要看定位了,我现在是独立了一个规则库,然后放到了 2 个地方:

  • eslint-config-egg 用于 lint 自动检测
  • egg-bin check 或者叫 egg-bin report 更合适,用于开发者提问时,我们怀疑他配置有问题,直接让他执行这条指令,再贴信息上来,免得多费口舌,有点像你的 tnpm network

而你提的 dev 和 prod,也可以做,就看有没有必要,要做也是在 egg-bin 和 egg-scripts 这一层,而不是 egg 那里,相当于 bin 强制做了一次 lint 了。

PS: 要改名为 egg-bin report 么?

@atian25
Copy link
Member Author

atian25 commented Feb 24, 2018

egg-bin check landed at 4.4.0

@atian25
Copy link
Member Author

atian25 commented Oct 10, 2018

19

@sunnylqm
Copy link

sunnylqm commented May 4, 2019

Nodejs的这个设计很不好(错的人多只能说明设计的错误),没有必要在文档中沿用这些混乱的写法。别说从其他语言转过来的,我写了七八年js,babel,webpack,eslint,tslint,metro的配置里从来没见过exports和module.exports的混写,所以我也不知道这些,也不会去记这些特定环境里的设定。 @atian25

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

No branches or pull requests

5 participants