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: MidwayRequestContainer 增加泛型标注 #407

Merged
merged 1 commit into from
Mar 1, 2020

Conversation

Lxxyx
Copy link
Member

@Lxxyx Lxxyx commented Feb 29, 2020

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

@czy88840616
Copy link
Member

要不直接搞2.0里吧。。

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #407   +/-   ##
=======================================
  Coverage   89.56%   89.56%           
=======================================
  Files          36       36           
  Lines         843      843           
  Branches       48       48           
=======================================
  Hits          755      755           
  Misses         78       78           
  Partials       10       10
Impacted Files Coverage Δ
packages/midway-core/src/requestContainer.ts 75% <ø> (ø) ⬆️

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 a564ca4...a66163c. Read the comment docs.

@Lxxyx
Copy link
Member Author

Lxxyx commented Feb 29, 2020

这个功能应该和版本无关,merge 之后1.x和2.x都行吧 @czy88840616

@czy88840616
Copy link
Member

也可以。。就是我要cherry-pick。。

@@ -43,7 +43,7 @@ export class MidwayRequestContainer extends MidwayContainer {
}
}

async getAsync<T>(identifier: any, args?: any) {
async getAsync<T = any>(identifier: any, args?: any) :Promise<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. TypeScript 中 T 默认就是any,这里没有必要加吧
  2. 默认就是返回 T 类型的

Copy link
Contributor

Choose a reason for hiding this comment

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

从 typescript 3.5.0 开始默认是 unknown 类型的。

Copy link
Member Author

@Lxxyx Lxxyx Mar 2, 2020

Choose a reason for hiding this comment

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

默认是 unknown,所以如果不加 T=any 的话,很多之前的老代码都可能会报错。 @kurten

Copy link
Member

Choose a reason for hiding this comment

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

any 既是 top 顶级类型也是 bottom 最下级类型,而 unknown 只是 bottom 最下级类型。

@czy88840616 czy88840616 merged commit daee9d3 into midwayjs:master Mar 1, 2020
czy88840616 pushed a commit that referenced this pull request Mar 1, 2020
feat: MidwayRequestContainer 增加泛型标注 (#407)
czy88840616 pushed a commit that referenced this pull request Mar 5, 2020
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.

6 participants