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

refactor: 重构前端代码 #988

Merged
merged 10 commits into from
Jan 22, 2024
Merged

refactor: 重构前端代码 #988

merged 10 commits into from
Jan 22, 2024

Conversation

PairZhu
Copy link
Contributor

@PairZhu PairZhu commented Jan 20, 2024

What does this PR do?

几乎完全重构前端js代码
小幅度重构后端代码(主要为了适配前端)

Motivation

优化前端代码,原先的代码逻辑过于混乱

Additional Notes

修改后端所有API为接受json格式
修复若干个命名风格混乱的问题
修复皮肤切换按钮的样式bug(原先只有黑夜模式有active动效)
将网卡信息直接嵌入html,可以减少请求次数,防止重复请求

几乎完全重构前端js代码
小幅度重构后端代码(主要为了适配前端)
@jeessy2
Copy link
Owner

jeessy2 commented Jan 20, 2024

改动也过大了吧。。。一个PR不宜过大,不方便review

  • 配置文件的属性名都改了,不兼容之前的配置。
  • jQuery是否引入
  • 网卡信息直接嵌入html,必须刷新页面才能看到变动

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 20, 2024

  1. 配置文件的属性名没有修改,修改的是后端接受的数据的key的名称
  2. 引入了jQery,但是这是值得的,之前引入的效果不佳是因为没有很好的应用jQuery的特性,jQuery作为工具必然有它的价值
  3. 网卡信息不会太频繁变动,用户不大可能长期停留在配置页面

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 20, 2024

这些改动都是连着的,这个pr就是为了彻底重构前端代码的,要拆分感觉比较麻烦。而且其实说的难听一些,之前的前端代码在一次一次的修改后已经堆成屎山了,我认为彻头彻尾的重构是必要的

@jeessy2
Copy link
Owner

jeessy2 commented Jan 20, 2024

重构还是可以。
之前把jQuery移除了,这次又加去,需要严格讨论是否有这个必要,不要搞些无效操作,主要是没看出来具体那点必须引入jQuery

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 20, 2024

jQuery本来就没有必要,任何前端框架都是没有必要的。但是引入后可以大大减轻开发者的负担,项目的复杂程度已经和之前不一样了,越高的复杂程度越需要框架,随着项目不断增加新功能引入框架是迟早的。也许等下次不得不重构时就需要引入vue这种更加高级的框架了

@jeessy2
Copy link
Owner

jeessy2 commented Jan 20, 2024

没必要啊,就一个稍微复杂一点点的页面,以后也不会有多大的改动。引入vue/react之类的完全是找事做

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 20, 2024

尤其是逻辑部分已经不是稍微复杂一点点了。这次重构花了我整整三四天的时间,因为目前代码的心智负担太重了,有些逻辑的耦合非常混乱和脆弱。反正我是非常建议引入框架的,引入框架不会有什么副作用不是吗?没有必要过度排斥框架。从另一个角度看,这个项目一样也引入了css框架不是吗?css框架和前端框架一样都是不必要的。只要框架没有带来无法忍受的副作用就没必要排斥啊

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 20, 2024

jQuery的大小我之前看错了,确实是80多KB

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 20, 2024

但是这个体积不论是对于内网还是外网都不算很大,不会影响网页打开速度

@jeessy2
Copy link
Owner

jeessy2 commented Jan 20, 2024

之前的代码也看得懂,也不是很复杂的代码,但是也可以重构一下代码,让更加看得明白。

如果项目没那个必要的话,我个人觉得没必要引入框架或库,看下原改动者有啥意见没有 @yin1999

还原了主题色修改函数的write参数,去除该参数会导致自适应修改主题时也会写入localStorage
自动修改”获取 IP 方式“标签的for属性,保证其始终指向当前可编辑的元素
@yin1999
Copy link
Contributor

yin1999 commented Jan 20, 2024

之前的代码也看得懂,也不是很复杂的代码,但是也可以重构一下代码,让更加看得明白。

如果项目没那个必要的话,我个人觉得没必要引入框架或库,看下原改动者有啥意见没有 @yin1999

感觉暂时还没有到必须要引入 jQuery 的地步,我的想法是可以分离一下原来的将js和html全部放到一起的写法,把主要的逻辑全部提取到单个js文件里面,然后对功能做一下逻辑封装(可以考虑使用 class),应该能够满足比较长的一段时间的发展需求了。

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 20, 2024

之前的代码也看得懂,也不是很复杂的代码,但是也可以重构一下代码,让更加看得明白。
如果项目没那个必要的话,我个人觉得没必要引入框架或库,看下原改动者有啥意见没有 @yin1999

感觉暂时还没有到必须要引入 jQuery 的地步,我的想法是可以分离一下原来的将js和html全部放到一起的写法,把主要的逻辑全部提取到单个js文件里面,然后对功能做一下逻辑封装(可以考虑使用 class),应该能够满足比较长的一段时间的发展需求了。

现在的情况是已经引入JQuery了,不需要更多的代价了去做这件事了,为什么引入框架需要考虑是否必要呢?我的理念是框架肯定是可以减少开发负担的,并且框架不会带来无法忍受的负面作用,那何乐而不为呢?
此外,我这次优化的原理主要也是通过html记号和修改js逻辑做的,引入jQuery只是附带的。

@yin1999
Copy link
Contributor

yin1999 commented Jan 21, 2024

现在的情况是已经引入JQuery了,不需要更多的代价了去做这件事了,为什么引入框架需要考虑是否必要呢?我的理念是框架肯定是可以减少开发负担的,并且框架不会带来无法忍受的负面作用,那何乐而不为呢? 此外,我这次优化的原理主要也是通过html记号和修改js逻辑做的,引入jQuery只是附带的。

本项目的前端逻辑仍旧足够简单,的确,对用户而言,引入 jQuery 并不存在明显的感知。所以我下面的说明也是从开发的角度去论述(每个人都会有自己的想法,所以下面的论述存在主观性):

  1. 引入 jQuery 意味着我们需要长期响应 jQuery 出现的各类漏洞,这是引入第三方库固然需要面对的。我们采用的是直接加入 jQuery 压缩后的 js 文件来进行版本控制,我不确定这能否被 GitHub 的代码扫描所覆盖。
  2. 不熟悉 jQuery 语法的开发者必须学习相关的语法才能维护相关的代码,存在较小的学习负担。很多人对于是否要学习 jQuery 存在争议,这就意味着有一部分前端开发者可能并不会去学习 jQuery。而原生 js + HTML5 作为前端的基础,是前端开发者必须学习的。在上一次移除 jQuery 之前,项目的代码中已经出现了 jQuery 和原生 js 混用的 DOM 操作逻辑。

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

现在的情况是已经引入JQuery了,不需要更多的代价了去做这件事了,为什么引入框架需要考虑是否必要呢?我的理念是框架肯定是可以减少开发负担的,并且框架不会带来无法忍受的负面作用,那何乐而不为呢? 此外,我这次优化的原理主要也是通过html记号和修改js逻辑做的,引入jQuery只是附带的。

本项目的前端逻辑仍旧足够简单,的确,对用户而言,引入 jQuery 并不存在明显的感知。所以我下面的说明也是从开发的角度去论述(每个人都会有自己的想法,所以下面的论述存在主观性):

  1. 引入 jQuery 意味着我们需要长期响应 jQuery 出现的各类漏洞,这是引入第三方库固然需要面对的。我们采用的是直接加入 jQuery 压缩后的 js 文件来进行版本控制,我不确定这能否被 GitHub 的代码扫描所覆盖。
  2. 不熟悉 jQuery 语法的开发者必须学习相关的语法才能维护相关的代码,存在较小的学习负担。很多人对于是否要学习 jQuery 存在争议,这就意味着有一部分前端开发者可能并不会去学习 jQuery。而原生 js + HTML5 作为前端的基础,是前端开发者必须学习的。在上一次移除 jQuery 之前,项目的代码中已经出现了 jQuery 和原生 js 混用的 DOM 操作逻辑。

基于我这几年来从事前端开发的经验和对近几年前端开发者技能情况的了解:

  1. jQuery又不是什么新兴的框架,已经是非常成熟的甚至可以说是陈旧的框架了,几乎不需要考虑框架造成的漏洞。此外如果要通过前端来保证数据或者服务器的安全性,这本身就是不合理的事情,前端代码只要功能正常即可。即便考虑前端漏洞造成的安全问题,那也只能是xss注入,但是我们代码中并没有通过url加载dom的操作。而且这类问题开发者使用原生框架也可能甚至可以说比使用框架更容易写出类似的有安全隐患的代码,这并不能算框架的缺点。如果你分析过市面上的网站,就会发现这些网站大部分都还使用jQuery框架,而且使用的版本都是相当老旧,说明他们很长一段时间没有更新过jQuery版本,这也侧面说明了jQuery的可靠性。
  2. jQuery框架是最接近原生的,几乎不用专门花时间精力去学习这个框架,而是阅读对应代码后自然就明白其用法,最多需要查查文档(使用原生js也需要查文档的不是吗?)。其次,现在学习前端几乎没有不学习框架的。近几年我几乎没见过从事前端开发的开发者只会原生js而不会框架的,而反而有很多从业者只会使用框架而不熟悉如何使用原生框架开发的。不过这个现象可能和jQuery没什么联系,因为这个框架相当老旧了,并且和原生js非常相近,大多数情况是开发者只会使用vue开发而不会原生或者jQuery开发。而且这个技能也算是一个门槛,如果对前端开发非常不熟悉的人随意提交pr被合并,那么项目还会像之前那样慢慢堆成屎山,原来的屎山大概率也是这么来的不是吗?最常见的问题就是各种命名风格混搭使用。

@yin1999
Copy link
Contributor

yin1999 commented Jan 21, 2024

你说的大部分我都赞同:更重要的是代码的质量。使用 jQuery 与否并不能直接提升整体的代码质量。项目的前端代码本身的复杂度也不是很高,我觉得引入 jQuery 与否其实更多的的看个人喜好。有人喜欢对代码的全面控制,喜欢使用由运行时环境提供的原生特性,有人喜欢用第三方库来简化代码调用逻辑。这本身没有对错优劣之分,就像 Turbo 的核心开发者选择移除 TypeScript 一样。

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

就我这次重构的代码来看,使用jQuery减少了很多循环嵌套操作,可以明显减少代码嵌套深度。jQuery的api可以做到一次对多个组件进行同样的操作。对于要频繁使用代码进行封装简化这个行为可能不算是提高代码质量,但是在我看来起码是更加优雅的操作。jQuery因为没有引入新的模型所以本质上就是帮你定义好很多常用的函数工具,如果世界上没有这个库的话让我自己开发项目的话我大概率也会做类似的封装。
此外,其实我还真的挺想用vue的,vue的双向绑定功能可以再大大减少这里面的代码量,让代码更加简洁。
在没有副作用的前提下,追求简洁和优雅不是好事吗?
不可否认,我也有一定的代码强迫症,会因为一段代码不够优雅而思考很久去想着如何重构。所以,如果项目的管理员无法接受这个变动(除非有充足的证据证明这样做有很大的弊端),我也不大愿意修改。毕竟确实有些理念不合,如果打扰到各位了我很抱歉。

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

另外我对后端不是非常了解,但是目前这个验证系统好像没办法注销登录,似乎也不会过期,这也算是一个安全隐患吧?身份信息在浏览器中还是使用base64明文保存的,这点也很容易造成信息的泄露

@yin1999
Copy link
Contributor

yin1999 commented Jan 21, 2024

另外我对后端不是非常了解,但是目前这个验证系统好像没办法注销登录,似乎也不会过期,这也算是一个安全隐患吧?身份信息在浏览器中还是使用base64明文保存的,这点也很容易造成信息的泄露

Chrome 浏览器还会明文存储用户名密码(大部分用户应该都会使用 Windows 来访问管理页面)。我相信好的使用习惯胜过大多数附加的安全措施。

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

另外我对后端不是非常了解,但是目前这个验证系统好像没办法注销登录,似乎也不会过期,这也算是一个安全隐患吧?身份信息在浏览器中还是使用base64明文保存的,这点也很容易造成信息的泄露

Chrome 浏览器还会明文存储用户名密码(大部分用户应该都会使用 Windows 来访问管理页面)。我相信好的使用习惯胜过大多数附加的安全措施。

还是有区别的,比如我把ddns暴露到公网中,然后临时使用别人的或者公共的设备访问的时候,那你的账号密码就会被存进别人的电脑还没法注销

@jeessy2
Copy link
Owner

jeessy2 commented Jan 21, 2024

还是有区别的,比如我把ddns暴露到公网中,然后临时使用别人的或者公共的设备访问的时候,那你的账号密码就会被存进别人的电脑还没法注销

用的Basic Authentication。是这效果,要不就单独做一个新的登录页面

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

还是有区别的,比如我把ddns暴露到公网中,然后临时使用别人的或者公共的设备访问的时候,那你的账号密码就会被存进别人的电脑还没法注销

用的Basic Authentication。是这效果,要不就单独做一个新的登录页面

能不能直接使用这个Authentication的值生成cookie然后自动删除Authentication,然后就不需要写登录页面了

@jeessy2
Copy link
Owner

jeessy2 commented Jan 21, 2024

关于jQuery。原生JS够强大了,可以在v站了解下,原生JS也能解决问题,只是稍微要长点:
$("#addBtn").click((e) => { -> document.querySelector("#addBtn").addEventListener("click", (e) => {

@jeessy2
Copy link
Owner

jeessy2 commented Jan 21, 2024

还是有区别的,比如我把ddns暴露到公网中,然后临时使用别人的或者公共的设备访问的时候,那你的账号密码就会被存进别人的电脑还没法注销

用的Basic Authentication。是这效果,要不就单独做一个新的登录页面

能不能直接使用这个Authentication的值生成cookie然后自动删除Authentication,然后就不需要写登录页面了

这个和浏览器有关,和cookie无关

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

关于jQuery。原生JS够强大了,可以在v站了解下,原生JS也能解决问题,只是稍微要长点: $("#addBtn").click((e) => { -> document.querySelector("#addBtn").addEventListener("click", (e) => {

jQuery能做到的原生都能做啊,但是jQuery少些了很多循环,你这是一个元素的监听器,如果是多元素的,jQuery还是这么写,但是原生就得写个循环或者forEach了。我既然是搞前端的,原生js肯定也是会的

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

还是有区别的,比如我把ddns暴露到公网中,然后临时使用别人的或者公共的设备访问的时候,那你的账号密码就会被存进别人的电脑还没法注销

用的Basic Authentication。是这效果,要不就单独做一个新的登录页面

能不能直接使用这个Authentication的值生成cookie然后自动删除Authentication,然后就不需要写登录页面了

这个和浏览器有关,和cookie无关

这个我理解错了,刚刚了解了一下Basic Authentication,这个信息不会持久化保存,浏览器关闭之后就没了,所以安全风险其实不高

@jeessy2
Copy link
Owner

jeessy2 commented Jan 21, 2024

关于jQuery。原生JS够强大了,可以在v站了解下,原生JS也能解决问题,只是稍微要长点: $("#addBtn").click((e) => { -> document.querySelector("#addBtn").addEventListener("click", (e) => {

jQuery能做到的原生都能做啊,但是jQuery少些了很多循环,你这是一个元素的监听器,如果是多元素的,jQuery还是这么写,但是原生就得写个循环或者forEach了。我既然是搞前端的,原生js肯定也是会的

还好吧,就多些字符,这样看到才更加容易理解,更加直观,可能我写react多些

@jeessy2
Copy link
Owner

jeessy2 commented Jan 21, 2024

按照这个逻辑,jQuery就没有存在的价值了,因为jQuery就是一堆语法糖罢了,但是jQuery曾经之所以被市场认可说明这些语法糖确实很有意义

简单项目还是原生好。想起10年前用起jQuery还是有点痛苦(项目比较复杂)

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

按照这个逻辑,jQuery就没有存在的价值了,因为jQuery就是一堆语法糖罢了,但是jQuery曾经之所以被市场认可说明这些语法糖确实很有意义

简单项目还是原生好。想起10年前用起jQuery还是有点痛苦(项目比较复杂)

那看来是你对jquery有阴影了,你要这么说的话我可不介意用vue再重构一遍,正好项目里很多表单都可以用上双向绑定[doge]

@jeessy2
Copy link
Owner

jeessy2 commented Jan 21, 2024

按照这个逻辑,jQuery就没有存在的价值了,因为jQuery就是一堆语法糖罢了,但是jQuery曾经之所以被市场认可说明这些语法糖确实很有意义

简单项目还是原生好。想起10年前用起jQuery还是有点痛苦(项目比较复杂)

那看来是你对jquery有阴影了,你要这么说的话我可不介意用vue再重构一遍,正好项目里很多表单都可以用上双向绑定[doge]

原生的吧,项目不复杂,没必要。
好多项目都不在依赖jQuery了,差点在2024年重新依赖jQuery😓

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 21, 2024

有什么项目是从jQuery转为原生的吗?多半不都是换框架吗?而且还是那句话,我认为框架不是有没有必要的事情。jQuery还是小型开发的框架首选之一。既然你实在接受不了框架,你看是你同意合并之后自己在当前基础上修改,还是我关闭这个pr你自己重构或者放弃重构计划。

@jeessy2
Copy link
Owner

jeessy2 commented Jan 21, 2024

有什么项目是从jQuery转为原生的吗?多半不都是换框架吗?而且还是那句话,我认为框架不是有没有必要的事情。jQuery还是小型开发的框架首选之一。既然你实在接受不了框架,你看是你同意合并之后自己在当前基础上修改,还是我关闭这个pr你自己重构或者放弃重构计划。

bootstrap5不依赖jQuery,还有其他一些。
要不你改下吧,其他重构还是挺好的

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 22, 2024

改好了。无法理解,为什么要把框架视为洪水猛兽呢。。。

@jeessy2
Copy link
Owner

jeessy2 commented Jan 22, 2024

改好了。无法理解,为什么要把框架视为洪水猛兽呢。。。

不是不用,是项目只有一个页面,太小了。没必要用

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 22, 2024

改好了。无法理解,为什么要把框架视为洪水猛兽呢。。。

不是不用,是项目只有一个页面,太小了。没必要用

反正我的理念是,框架不是非得有必要才能用

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 22, 2024

这次的pr先做这些,下面就是测试和review了。增加测试之类的下个pr再做

web/writing.html Outdated Show resolved Hide resolved
@jeessy2
Copy link
Owner

jeessy2 commented Jan 22, 2024

这些命名需要统一否? account_help Ipv4UrlHelp

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 22, 2024

这些命名需要统一否? account_help Ipv4UrlHelp

统一了我写的一部分,其余的有些懒得挨个改了🤦‍

@jeessy2
Copy link
Owner

jeessy2 commented Jan 22, 2024

之前默认有多个配置的话,首次进入会跳到最后一个,这样才能看到共多少个配置。

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 22, 2024

之前默认有多个配置的话,首次进入会跳到最后一个,这样才能看到共多少个配置。

没明白?是需要默认选最后一个配置的意思?

@jeessy2
Copy link
Owner

jeessy2 commented Jan 22, 2024

之前默认有多个配置的话,首次进入会跳到最后一个,这样才能看到共多少个配置。

没明白?是需要默认选最后一个配置的意思?

对。进入页面,默认加载最后一个。才会显示10-xxx,才容易看出来有多少个配置。现在默认是第一个

@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 22, 2024

之前默认有多个配置的话,首次进入会跳到最后一个,这样才能看到共多少个配置。

没明白?是需要默认选最后一个配置的意思?

对。进入页面,默认加载最后一个。才会显示10-xxx,才容易看出来有多少个配置。现在默认是第一个

这个改好了,这改动比较大要不再找上次给我pr review的大佬也review一下以防万一?

Copy link
Owner

@jeessy2 jeessy2 left a comment

Choose a reason for hiding this comment

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

还有一些建议项,比如日志之类的,单独在放一个js文件?

static/constant.js Outdated Show resolved Hide resolved
static/i18n.js Outdated Show resolved Hide resolved
web/writing.html Outdated Show resolved Hide resolved
@PairZhu
Copy link
Contributor Author

PairZhu commented Jan 22, 2024

还有一些建议项,比如日志之类的,单独在放一个js文件?

不太合适感觉,因为这不是模块,不是工具包不可能被复用,分开放没有太大的意义。另外我不太确定变成url之后会不会影响执行顺序

@jeessy2
Copy link
Owner

jeessy2 commented Jan 22, 2024

还有一些建议项,比如日志之类的,单独在放一个js文件?

不太合适感觉,因为这不是模块,不是工具包不可能被复用,分开放没有太大的意义。另外我不太确定变成url之后会不会影响执行顺序

那先不管,在仔细看下有没有没review到的

web/writing.html Outdated Show resolved Hide resolved
@jeessy2 jeessy2 merged commit 7befe6e into jeessy2:master Jan 22, 2024
2 checks passed
@jeessy2
Copy link
Owner

jeessy2 commented Jan 22, 2024

thanks @PairZhu

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.

None yet

3 participants