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

fix(boilerplate) update plugin declaration #295

Merged
merged 3 commits into from
Jul 29, 2019

Conversation

hzgotb
Copy link
Contributor

@hzgotb hzgotb commented Jul 24, 2019

Checklist

  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@codecov-io
Copy link

codecov-io commented Jul 24, 2019

Codecov Report

Merging #295 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #295   +/-   ##
=======================================
  Coverage   86.61%   86.61%           
=======================================
  Files          35       35           
  Lines         695      695           
  Branches       48       48           
=======================================
  Hits          602      602           
  Misses         83       83           
  Partials       10       10

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c695ce0...4f51473. Read the comment docs.

@@ -1,4 +1,4 @@

import { EggPlugin } from 'midway'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

加个分号吧。

Copy link
Member

@waitingsong waitingsong Jul 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我以前是分号党以及4space党。现在转成无分号党以及2space党了。
不用分号是利大于弊,比如链式操作,开发测试阶段有时候需要暂时注释掉某行(比如末尾行)。如果ide自动添加分号,那么在改回去之后就会导致异常,还得删除掉。

个人建议使用无分号风格。毕竟

  • lint工具会检查(无分号时)语义。
  • TS 编译到 js 后是会自动添加分号的。

所以对于 TypeScript 项目以及带有 eslint 检查的 JS 项目来说 ASI 基本上不存在任何问题了。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 1、这个就涉及到整体的团队规范了,如果是个人风格完全没有问题的,但是涉及到团队或者多人协作的时候还是保持一个统一的结构和lint规则,有利于推进,协作。
  • 2、这个纷争就和编辑器一样经久不衰。。 涉及到语义、可读性等等,在没有结论的时候,跟着大厂走会比较好,我们参考过微软的标准,所以产生了这份 lint。

Copy link
Member

@waitingsong waitingsong Jul 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

十年前我用分号和 tab (8位),基本没做 lint。
因为tab在不同查看工具下风格迥异,于是改成 4space。
谷歌提出 2space风格后,觉得样式不可接受,因为 2space 在 vim 下显示效果过于紧凑。
后来上了 TS 于是主要使用 vscode,这时候 2space 显示效果就可以接受了。
随着 jQuery 带来的链式调用普及,lint --fix 自动添加分号导致开发调试经常因为分号带来不便,于是终于抛弃了分号。
其实这些变化是随着开发工具的提升而变化的。以前强调、制订的风格、规则是因为工具跟不上需求所以只能靠规范来约束,现在随着 Node.js 带来的(前端)自动化工具的飞跃,很多事情就可以依靠工具(或者说配置规则)来完成了。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我是比较建议上加分号。但是lint自动没分号。我就没加了。包括带eslint的模板,都是没分号的。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

脚手架的目录可能不在项目整个 lint 范围内,提示不一定准确,如果脚手架里是加的,就加上,我看了下只有 midway-ts-strict-boilerplate 这个脚手架配的 lint 没有,可以不加。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

微软应该也会转成无分号党的 😄
前两天在 github Microsoft/TypeScript 库上搜索 issue ,发现一个 2017 年的需求就要实现就是:
在自动 import 时可以根据lint配置的分号规则来决定行尾是否有分号~~

Copy link
Member

@waitingsong waitingsong Jul 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

微软应该也会转成无分号党的 😄
前两天在 github Microsoft/TypeScript 库上搜索 issue ,发现一个 2017 年的需求就要实现了:
在 vscode 中写代码,自动 import 添加导入行时,可以根据lint配置的分号规则来决定行尾是否有分号~~

@czy88840616
Copy link
Member

image

@waitingsong
Copy link
Member

waitingsong commented Jul 25, 2019

我这儿没问题:

2019-07-25_113156

不过对于 nunjucks/cors 无自动完成功能(貌似 EggPluing 类型上无此属性)。
security static 可自动完成。

@czy88840616
Copy link
Member

昨天看了下,那个能力似乎是 ts-helper 自动生成 d.ts 来解决的,触发源还没看到在哪,可能是vscode插件里。

@hzgotb
Copy link
Contributor Author

hzgotb commented Jul 25, 2019

解决插件在config里能出现配置,我是通过在Modal配置里直接导入包解决。

import 'egg-jwt'

这样。

还有就是建议把一些egg的定义也放出来。例如 IgnoreOrMatch 。有一些插件没跟上 egg 的更新,少了这两个属性。

@czy88840616
Copy link
Member

czy88840616 commented Jul 25, 2019

@hzgotb 可以放出来的,之前导了几个常用的,直接全导貌似有冲突,只能一个个来。 你直接提了呗。。

@czy88840616 czy88840616 merged commit 4542932 into midwayjs:master Jul 29, 2019
czy88840616 pushed a commit that referenced this pull request Jul 29, 2019
fix(boilerplate) update plugin declaration (#295)

fix(boilerplate) update plugin declaration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants