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: using egg-typebox-validate for PackageController #12

Merged
merged 4 commits into from
Dec 3, 2021

Conversation

xiekw2010
Copy link
Contributor

在 ts 场景下,使用 egg-typebbox-validate 来替换 egg-validate

@fengmk2 fengmk2 added enhancement New feature or request good first issue Good for newcomers labels Dec 2, 2021
const FullPackageRule = Type.Object({
name: Type.String(),
// Since we don't validate versions & _attachments previous, here we use Type.Any() just for object validate
versions: Type.Record(Type.String(), Type.Any()),
Copy link
Contributor Author

@xiekw2010 xiekw2010 Dec 2, 2021

Choose a reason for hiding this comment

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

这里为什么不对 versions 和 _attachments 也写明确的 TypeBox 定义呢?

如果写了,也就是标准的 json-schema 了,对于这个 controller 里用到的代码,对于 testcase 里的非法参数,在 ctx.tValidate 的时候就被被拦截掉。不用等到下面具体代码再去拿出值来做判断,但是 test case 的 assert 是要跑到的,所以这里为了改动小一点,临时就这写 any 了。

长期来看,所有校验都可以通过 TypeBox 写的 json-schema 来做处理,不用下面代码再去拿出来做二次校验。

Copy link
Member

Choose a reason for hiding this comment

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

嗯,先不在这里校验,里面的校验逻辑目前还没有固化,涉及很多历史 client 版本,后续我将所有 client 上报数据格式都测试之后,再来加到这里。

@xiekw2010 xiekw2010 changed the title [WIP]feat: using egg-typebox-validate for PackageController feat: using egg-typebox-validate for PackageController Dec 3, 2021
@fengmk2 fengmk2 merged commit 9a8cf96 into cnpm:main Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants