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

feat: 使用eslint规范代码, 城市筛选添加本地缓存 #57

Closed
wants to merge 5 commits into from

Conversation

Night-stars-1
Copy link
Contributor

@Night-stars-1 Night-stars-1 commented Mar 22, 2024

Description

城市筛选添加本地缓存->已在本地测试通过

onMounted(() => {
blockCities.value.forEach(cityName => {
selectedCity.value = selectedCity.value.filter(city => city.name !== cityName)
})
})

Linked Issues

resovle #56

@yjl9903
Copy link
Owner

yjl9903 commented Mar 22, 2024

功能性修改单独提 PR

@yjl9903
Copy link
Owner

yjl9903 commented Mar 22, 2024

image

最后一个逗号关闭,行末的分号打开 -> 个人习惯

@yjl9903
Copy link
Owner

yjl9903 commented Mar 22, 2024

autofix.ci 修改为使用 eslint

- name: Format
run: pnpm format

@Night-stars-1
Copy link
Contributor Author

功能性修改单独提 PR

这个修改我直接提pr是因为代码很简单,只有我几行

chore: 微调部分代码,使其符合eslint规则
chore: autofix使用eslint
@Ximu-Luya
Copy link
Collaborator

eslint格式化的代码果然令人顺心许多

@Ximu-Luya
Copy link
Collaborator

Ximu-Luya commented Mar 22, 2024

@Night-stars-1 需要在ESlint中添加规则,详情请看ESlint - semi

@yjl9903 要不要分号在一念之间哈哈哈,不过我个人确实更倾向于不用分号,都2024年了(

chore: 微调部分代码,使其符合eslint规则
chore: autofix使用eslint
@Ximu-Luya
Copy link
Collaborator

功能性修改单独提 PR

这个修改我直接提pr是因为代码很简单,只有我几行

@Night-stars-1 另外建议功能更新之间独立一点,毕竟你这个ESlint的添加,文件变化比较大,个人认为最好页面功能与非页面功能区分开。

@Night-stars-1
Copy link
Contributor Author

@Night-stars-1 需要在ESlint中添加规则,详情请看ESlint - semi

@yjl9903 要不要分号在一念之间哈哈哈,不过我个人确实更倾向于不用分号,都2024年了(

已经更改了,eslint的semi不支持ts,需要用typescript-eslint
添加分号不是好文明,更改解决更改冲突花了好长时间,太容易冲突了,合并困难

@Night-stars-1
Copy link
Contributor Author

功能性修改单独提 PR

这个修改我直接提pr是因为代码很简单,只有我几行

@Night-stars-1 另外建议功能更新之间独立一点,毕竟你这个ESlint的添加,文件变化比较大,个人认为最好页面功能与非页面功能区分开。

嗯,这个提交是因为内容简单就合一起了

@yjl9903
Copy link
Owner

yjl9903 commented Mar 22, 2024

@Night-stars-1 即使非常简单也不要合在一起

@Night-stars-1
Copy link
Contributor Author

@Night-stars-1 即使非常简单也不要合在一起

删除了

@Ximu-Luya
Copy link
Collaborator

Ximu-Luya commented Mar 22, 2024

已经更改了,eslint的semi不支持ts,需要用typescript-eslint

我看你的提交里已经有typescript-eslint插件了,不过插件里的分号规则也弃用了。可以找到的是 @stylistic/ts/semi

不过有意思的是,看起来当前提交的diff里,分号已经被去除了,这是因为推荐规则中自带的原因吧,这算是官方推荐了?(笑

@Night-stars-1
Copy link
Contributor Author

已经更改了,eslint的semi不支持ts,需要用typescript-eslint

我看你的提交里已经有typescript-eslint插件了,不过插件里的分号规则也弃用了。可以找到的是 @stylistic/ts/semi

不过有意思的是,看起来当前提交的diff里,分号已经被去除了,这是因为推荐规则中自带的原因吧,这算是官方推荐了?(笑

我弃用的是eslint的semi,使用typescript-eslint扩展的semi了适配ts
不过typescript-eslint的semi已经被弃用了,由于本项目没有引用stylistic,单独为了semi引用有点不合适
typescript-eslint的semi已经可以使用了

@Ximu-Luya
Copy link
Collaborator

@Night-stars-1 不好意思,Ctrl + F搜索错了,搜semicolon没搜到规则,我误以为没加,我的问题

@yjl9903
Copy link
Owner

yjl9903 commented Mar 22, 2024

唉 好麻烦 这就是我为什么不想搞 eslint

@Night-stars-1
Copy link
Contributor Author

@Night-stars-1 不好意思,Ctrl + F搜索错了,搜semicolon没搜到规则,我误以为没加,我的问题

现在很多项目已经不加分号了
standardjs规范也推荐不加分号,

Comment on lines 11 to 12
FormMessage,
} from '@/components/ui/form'
Copy link
Owner

Choose a reason for hiding this comment

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

去除最后一个逗号

保留分号

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个ESlint规则可配置

Comment on lines 25 to 30
const percent = form.values.percent?.[0] ?? 100
if (type === 'add')
form.setFieldValue('percent', [percent + 1])
else
form.setFieldValue('percent', [percent - 1])
}
Copy link
Owner

Choose a reason for hiding this comment

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

当行也要保留大括号

Comment on lines +128 to +130
<Button type="submit">
上报
</Button>
Copy link
Owner

Choose a reason for hiding this comment

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

这个语义一致吗,会多空格吗?

Copy link
Collaborator

Choose a reason for hiding this comment

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

多空格其实也不影响,不过这个换行确实有点奇怪,ESlint应该也是可配置的

else {
const tr = product.transactions.find(tr => tr.targetCity === target)
if (tr)
form.resetField('price', { value: tr.basePrice })
Copy link
Owner

Choose a reason for hiding this comment

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

else 为什么掉下来了

Comment on lines 135 to 140
if (percent === undefined)
return
if (form.isFieldDirty('price'))
return
if (!form.values.targetCity)
return
Copy link
Owner

Choose a reason for hiding this comment

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

包括上面,提前的 return 我不喜欢它给我换行

const sourceCityName = getRouterParam(event, 'source');
const productName = getRouterParam(event, 'name');
const targetCityName = getRouterParam(event, 'target');
const id = getRouterParam(event, 'id');
if (!sourceCityName || !productName || !targetCityName || !id || !/^\d+$/.test(id)) {
setResponseStatus(event, 400);

Copy link
Owner

Choose a reason for hiding this comment

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

它这里为什么加了个空行?

Copy link
Collaborator

Choose a reason for hiding this comment

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

可能有配置是在方法的return语句前需要一个空行

const startGetData = async () => {
await fetch();

// 每隔 10 秒重新获取数据
Copy link
Owner

Choose a reason for hiding this comment

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

@Ximu-Luya try-catch 一下

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个我之后处理一下吧

@@ -1,27 +1,27 @@
import type { CityInfo, ProductInfo } from './types';

// @ts-ignore
// eslint-disable-next-line import/extensions
Copy link
Owner

Choose a reason for hiding this comment

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

怎么把 ts-ignore 给删了

Copy link
Collaborator

Choose a reason for hiding this comment

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

因为有eslint-disable-next-line这个了吧

presetTypography,
presetUno,
presetWebFonts,
Copy link
Owner

Choose a reason for hiding this comment

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

有机会我会自己写我喜欢的 import eslint 插件,所有不要让它动 import

Copy link
Collaborator

Choose a reason for hiding this comment

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

这个看起来是把import按字母顺序排序了

}

export interface ProductInfo {
city: string;
city: string
Copy link
Owner

Choose a reason for hiding this comment

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

这个分号我也是要的

Copy link
Collaborator

Choose a reason for hiding this comment

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

我个人理解是能不要分号就不要分号,分号一般只在单行多条语句使用。不过你要是强烈要求要加分号,把ESlint那分号规则设置为always就解决了

@yjl9903
Copy link
Owner

yjl9903 commented Mar 22, 2024

@Night-stars-1 不好意思,Ctrl + F搜索错了,搜semicolon没搜到规则,我误以为没加,我的问题

现在很多项目已经不加分号了 standardjs规范也推荐不加分号,

我扫了一下你发的这个标准,和它的引用,基本也就说了 2 件事:介绍 ASI 的原理,现代浏览器都正确实现了 ASI。或许大家都觉得没有逗号看起来舒服点,但是似乎也没给充足地、致命地拒绝加分号的理由。

那么,我多解释一下我个人的习惯:我本来就在自然语言的 writing 里,对自己的标点符号有比较严格的、一致性的要求。如何使用标点,是否使用标点,是肌肉记忆地就会对其仔细考量。显式地加在每一句话后加标点(这里是分号),对我来说是重要的。

对比其它编程语言,比如 Rust 最令我印象深刻,给了不加分号另外的语义(返回值),我是无法接受的,我更倾向于都使用标点和显示地 return

这是我个人的看法、喜好和习惯,如果要拒绝这个风格,可能需要和我解释更多致命的缺陷。

@Ximu-Luya
Copy link
Collaborator

@yjl9903 oh my god,你review评论了真多,我大都回复了。整体看了一下你提到的点,都可以解决。这个朋友的eslint配置,除了使用推荐规则以外,还多了非常多单独的规则,import 的排序、变量命名前面加下划线、if else换行和大括号问题,都不在推荐规则内,所以大的痛点都可以解决。

分号这个问题,没有什么致命的缺陷,确实就是个人习惯问题。我主要写脚本语言,Python、JavaScript和Typescript都可以不叫分号,所以我的习惯来自于此。不过ESlint规则一个简单配置的事,我倒是不至于不加分号会死(公司里的屎山代码早看惯了),有ESlint自动帮我处理一下倒是问题不大。

@Ximu-Luya
Copy link
Collaborator

@Night-stars-1 哈喽,你Close了这个PR,请问还会提新的吗?如果你没有这个打算的话,我可能就自己明后天来配置ESlint规则了。

@Night-stars-1
Copy link
Contributor Author

@Night-stars-1 哈喽,你Close了这个PR,请问还会提新的吗?如果你没有这个打算的话,我可能就自己明后天来配置ESlint规则了。

不加了众口难调,这些规范还是你们自己写吧,之后我提交一下本地缓存设置吧

@Night-stars-1
Copy link
Contributor Author

关于分号和空格,很多项目都已经是这样做的了
就比如这个项目的nuxt就没有加分号,代码段之间增加空格,这样对可读性有帮助
else有助于折叠

@yjl9903
Copy link
Owner

yjl9903 commented Mar 22, 2024

关于分号和空格,很多项目都已经是这样做的了 就比如这个项目的nuxt就没有加分号,代码段之间增加空格,这样对可读性有帮助 else有助于折叠

@Night-stars-1

  1. 别人怎么做,所谓的热门的项目怎么做,和我的喜好、我怎么做没有关系哈。就比如 vue / nuxt / ... 一直是忽略分号,antfu 发博客阐述自己不用 prettier 的原因,我尊重大家的选择,也能理解它们选择的原因,但是要成为我的实践那应该是我要认可的,best practice 要成为 my practice 最关键的是 practice 本身以及它和我之间的关系,不是任何为它背书的某种 "权威",甚至包括诸如 Linux 的某些代码风格(缩进,嵌套深度的要求...)。
  2. 代码段之间增加空行确实有助于可读性,但是在上面的 review 里我的疑问是 “为什么本来没空行,它添加了空行?”,疑问的点在于它为什么添加空行,应用了什么的规则?
  3. else 下放确实有助于代码折叠,尝试了 vscode 和 intellij,基本都会有类似的问题,搜索了 Jetbrains 相关的 issues(IDEA-314952, IDEA-199618,206379219),最早追溯到了 14 年以前,但似乎都没有太多进展,vscode 没有直接搜到相关的。但是可能 argue 点在于,我似乎从来没注意到这个问题,或者说注意到了但并没有非常打断我的 flow,为此改变七八年来的习惯似乎对我来说事实上不可能。

关于我对 ESLint 的看法:我一直想配自己用的,但是由于它的复杂性,以及对我个人而言的优先度很低,所以从来没有研究过。

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

Successfully merging this pull request may close these issues.

3 participants