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: allow inject proto and name #40

Merged
merged 3 commits into from
Jul 1, 2022
Merged

Conversation

whxaxes
Copy link
Member

@whxaxes whxaxes commented Jul 1, 2022

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

优化 Inject 逻辑,让 Inject 更简单易用,可以直接 Inject name ,也支持通过 design:type 来获取 protoClass

@ContextProto()
export class TestService {
  sayHi() {
    console.info('hi');
  }
}


@ContextProto()
export class TestService2 {
  sayHi() {
    console.info('hi');
  }
}

@ContextProto()
export default class CacheService {
  static fileName = __filename;

  @Inject({
    name: 'fooCache',
  })
  cache: ICache;

  @Inject('testService')
  testService: TestService;

  @Inject()
  otherService: TestService2;
}
Description of change

@whxaxes whxaxes requested a review from killagu July 1, 2022 05:04
@codecov
Copy link

codecov bot commented Jul 1, 2022

Codecov Report

Merging #40 (da341fd) into master (8e2a738) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   92.00%   92.02%   +0.02%     
==========================================
  Files         196      196              
  Lines        3950     3960      +10     
  Branches      378      384       +6     
==========================================
+ Hits         3634     3644      +10     
  Misses        315      315              
  Partials        1        1              
Impacted Files Coverage Δ
core/core-decorator/src/decorator/Inject.ts 100.00% <100.00%> (ø)
core/core-decorator/src/util/MetadataUtil.ts 89.47% <100.00%> (ø)
core/core-decorator/src/util/PrototypeUtil.ts 100.00% <100.00%> (ø)
core/eventbus-runtime/src/SingletonEventBus.ts 77.08% <0.00%> (+0.99%) ⬆️

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 8e2a738...da341fd. Read the comment docs.

@killagu
Copy link
Contributor

killagu commented Jul 1, 2022

  @Inject(TestService)
  testService: TestService;

这样代码太冗余了。

@whxaxes
Copy link
Member Author

whxaxes commented Jul 1, 2022

改成只支持传 name 的简写了

@whxaxes
Copy link
Member Author

whxaxes commented Jul 1, 2022

@killagu 再看一下?

@Inject()
foo: Foo;
export class Foo extends Base {

Copy link
Member Author

@whxaxes whxaxes Jul 1, 2022

Choose a reason for hiding this comment

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

加了 emitDecoratorMetadata ,下面的引用编译成 js 也实际存在了,所以这里顺序要改到引用前面

@whxaxes whxaxes force-pushed the feat-allow-inject-proto branch from ea43087 to da341fd Compare July 1, 2022 07:26
@whxaxes
Copy link
Member Author

whxaxes commented Jul 1, 2022

加上 design:type 的支持了,@killagu 再看一下?

Copy link
Contributor

@killagu killagu left a comment

Choose a reason for hiding this comment

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

LGTM

@whxaxes whxaxes merged commit abd1766 into master Jul 1, 2022
@whxaxes whxaxes deleted the feat-allow-inject-proto branch July 1, 2022 07:37
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.

2 participants