文章目录
- 前言
- Bug 重现
- 问题排查
- 解决方案
- old version
- new version
- 收到回复
- PR提交完整过程
- fork仓库 & clone代码仓
- 添加upstream
- fetch 新分支
- 创建新分支
- 完成修改(注意commit规范)
- push到个人仓库
- 创建Pull Request
- 填写PR信息
前言
G6正在进行v4
到v5
的版本升级,发了几个Issue Hunt,因为很喜欢G6,所以想尝试帮助完成一个矩形Item的迁移。在编写测试demo的过程中,发现了G6的一个严重bug。本文记录了我从发现bug,排查bug到给G6提PR,与仓库管理员沟通,最终PR被成功merge的过程。这是我给G6的第二个PR,给想要参与开源,为自己喜欢的项目贡献绵薄之力的朋友提供一套完整的贡献流程参考。附上两次Pull Request的链接
- Doc fix :Fix issue#4552, another 404 not found and typo errors #4554
- Bug fix :Fix: “Node not found” error from ‘getNode()’#4608
Bug 重现
这是我在编写测试demo时,发现G6中存在的一个bug。报错信息显示:Node not found for id: 1
。
根据字面意思,某个方法收到了id
为1的节点,但是在我传入的数据中并不存在这个节点。
问题排查
我在Graph.getNode()
这个方法的前后调试了很久,更奇怪的是,直接调用Graph.getNode(1)
居然是能够返回节点的。由于G6的代码中写的是:
Object.keys(data).forEach((id)=>{
//...
const innerModel = graphCore.getNode(id);
//...
})
我在getNode之前在控制台console.log
了id
, 控制台输出1
。这就很奇怪了,我一度怀疑是getNode
这个方法出现了问题,但是getNode
是一个核心方法,应该不可能出错。
如果直接调用Graph.getNode(1)
能够返回节点,那么说明id就不是1,于是我输出了id==1
,果不其然控制台输出false
。进一步使用typeof
查看id
的类型,才发现id不知道怎么已经变成了string
。原来使用Object.keys()
生成的数组,无论key
的类型是什么,统一生成为string
数组。这个bug
很严重啊,如果用户在数据中定义的id
是number
类型,那么将无法获取到这个Node
。
到这里,问题就定位完毕了。
解决方案
其实要解决这个问题很简单,有以下三种方法:
- 在用户的id为
number
类型时,使用Number(id)
进行一次转换,而在用户id
为string
类型时,不做任何处理 - 修改
getNode()
,使他能够识别用户传入数据中id
的类型 - 在文档中强制限制用户输入
string
类型,并且使用类型检查将用户输入的data限制为string
三种方法首先排除第二种,因为getNode()
是一个核心方法,是从antv
的核心代码仓中import
过来的一个方法,找不到修改的入口
我这里选择了第一种方法,并在PR中提示了,如果不做修改的话需要在文档中明确标注id
必须为string
类型
在解决这个问题的时候,我还考虑到了一种情况,如果用户比较调皮,输入的id
中又有string
类型,又有number
类型应该怎么解决呢?这里我采用了添加try-catch
代码块来进行解决。
old version
// 'id' variable is always string in here, but one in user data is number, possibly.
Object.keys(data).forEach((id)=>{
const innerModel = graphCore.getNode(id);
const relatedEdgeInnerModels = graphCore.getRelatedEdges(id);
})
new version
Object.keys(data).forEach((id)=>{
let innerModel
try {
innerModel = graphCore.getNode(id);
} catch (e) {
innerModel = graphCore.getNode(Number(id))
}
let relatedEdgeInnerModels;
try {
relatedEdgeInnerModels = graphCore.getRelatedEdges(id);
} catch (error) {
relatedEdgeInnerModels = graphCore.getRelatedEdges(Number(id));
}
})
并且我在我的PR底下,做了如下说明,来询问这么做是否合理:
An error is throwed from this function ‘getNode()’; msg: “Node not found for id: 1”
After testing, I found that ‘id’ variable is always string afterObject.keys(update).forEach((id)=>{...})
, but one in user data is number, possibly.
I tried adding atry-catch
block to fix this bug, but it doesn’t seem very reasonable to do so. I guess you can restrict the user to set theid
to a string type in the document, or force theid
to a string type in thetransformer data
layer to avoid this error.
getNode()
方法抛出了一个异常; 报错信息为:“找不到 id为1的节点“
经过测试,我发现id
变量在Object.keys(update).forEach((id)=>{...})
之后总是字符串类型,但是这个id
变量在用户数据中很有可能是数字类型。
我尝试添加一个try-catch
块来修复这个错误,但这样做似乎不太合理。 我想你们可以在文档中说明:限制id
为字符串类型,或者在transformer data
层强制将id
设置为字符串类型来避免这个错误。
收到回复
很快,我收到了仓库管理员十吾的回复,她回复了一个👍,我好开心,我问她这是可以接受的吗,如果是的话,需不需要重新创建一个PR来进行提交(因为我一开始提交的PR有其他修改,但是另外的修改无法被merge)。
PR提交完整过程
这一部分记录完整的PR提交过程,其中包含了我遇到的问题,一并做陈述并给出解决方案。因为这是我第二次给开源仓库做贡献,所以一些看起来很简单的细节我也记录在这里,帮后面的同学少踩一些坑。
fork仓库 & clone代码仓
直接fork,选仅fork默认分支即可。fork仓库后,在自己的github主页就能看到一个一摸一样的代码仓了。这一步注意,是要clone自己fork后的代码仓,比如我需要clone的地址是:https://github.com/zqqcee/G6.git
,这里zqqcee
是自己的用户名,不要clone错了。
添加upstream
这一步的目的是将antvis
的源仓库添加为上游仓库,不然我们无法同步它们的更新。运行:
git remote add upstream "https://github.com/antvis/G6.git"
运行完毕后,输入git remote -v
,能够看到
origin:xxxx
origin:xxxx
upstream:xxxx
upstream:xxxx
fetch 新分支
由于我是给v5分支提的PR,因此我需要先fetch v5分支。运行:
git fetch upstream/v5
输入git branch -a
就可以看到有一个红色的分支upstream/v5
,这说明已经fetch成功了
下一步,我们就需要把这个分支的内容在本地创建,并进行修改。
创建新分支
这一步在我执行的时候有一个很大的坑:我在master分支上直接运行:
git checkout -b v5
git rebase xxx
结果出了一堆冲突,后来才知道是我的v5分支是从master分支上创建的,而不是从远程拉过来的。
应该输入:
git checkout -b origin/v5 upstream/v5
这一步的意思是从upstream/v5
分支创建一个origin/v5
分支。
到这里还没结束,因为这个origin/v5
分支是我们从upstream中拉取出来的,我的习惯是要在这个分支上再新建一个分支做开发,分支名也有一些含义,于是接着运行
git checkout -b v5-fix#NodeNotFound
完成修改(注意commit规范)
写完代码后注意自己的commit规范,每一个commit都要让别人能看懂,不要全部修改完再做提交。这里我把每一个修改的含义都分得比较清楚,如下:
- bug 重现 commit
- bug 修复 commit
push到个人仓库
在这一步我遇到了大麻烦,由于G6发布了issue hunt
,因此这里我push到个人仓库时,由于我的personal token
没有包含workflow,因此push不成功。报错:“refusing to allow a Personal Access Token to create or update workflow .github/workflows/build.yml
without workflow
scope”
这一步正确的解决方案是重新创建一个Token并push,但是我为了省事,直接把workflow删掉了。结果PR就没有被合,并且收到了这个comment:
对于这个问题,解决方案我也记录在此处:
-
首先,创建一个token,勾选workflow,这一步在网上有很多教程,跟着做就好了,这里不做过多赘述
-
接着,重新设置
origin
。运行git remote remove origin git remote add origin https://{token}@github.com/zqqcee/G6.git
将
{token}
替换为刚刚创建的带workflow的token -
最后,重新push就能成功了
创建Pull Request
到fork的仓库中,push成功后,仓库中会显示有一个新的分支。然后点击Pull Request
创建一个新的Pull Request
这一步没什么好说的,重点是要选对你要修改的分支
填写PR信息
PR信息非常关键,必须非常清楚地说明你为什么要创建这个PR ,以及这个PR修复了什么问题。这里直接贴上我的PR 说明,供参考。
以上就是全部的解决过程了,很开心能为G6做了贡献,希望有机会能加入AntV团队,也希望自己能为更多仓库创建更优秀的PR~!