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

[RFC] 精简 core 对外透出的装饰器 #190

Closed
hyj1991 opened this issue Sep 6, 2022 · 17 comments · Fixed by #193
Closed

[RFC] 精简 core 对外透出的装饰器 #190

hyj1991 opened this issue Sep 6, 2022 · 17 comments · Fixed by #193
Assignees

Comments

@hyj1991
Copy link
Member

hyj1991 commented Sep 6, 2022

可以保留基类,去除 application 上的引用,上游继承后直接 Inject 基类或者实现类来通过 IoC 的标准形式进行引入。

@noahziheng
Copy link
Member

负面影响:

对于同一实现需要覆盖时,需要用户主动使用 Id 机制替代,如 RFC 例中的 Logger,此前允许用户通过下述代码获得唯一的 Logger:

@DefineLogger()
class InternalLogger implements Logger {}

@DefineLogger()
class ExternalLogger  implements Logger {}

// --

@Inject(ArtusInjectEnum.Logger)
logger: Logger;

在完成改动后,下述场景必须明确注入:

@Injectable()
class InternalLogger {}

@Injectable()
class ExternalLogger {}

// ---

@Inject()
logger: InternalLogger;

// or
@Inject()
logger: ExternalLogger;

// or
@Inject()
logger: ArtusLogger;

如仍需达成覆盖效果,必须由上层用户自己定义 Id(或保留 Artus 内的 Symbol):

const LOGGER_SYMBOL = Symbol.for('logger');

@Injectable({ id: LOGGER_SYMBOL })
class InternalLogger  implements Logger  {}

@Injectable({ id: LOGGER_SYMBOL })
class ExternalLogger  implements Logger  {}

// ---

@Inject(LOGGER_SYMBOL)
logger: Logger;

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

这种确实也是问题,id + 基类,可以做到近似同一份虚拟类 + 不同实现的能力

@atian25
Copy link
Member

atian25 commented Sep 6, 2022

@hyj1991 框架覆盖下层的一个实现这个的用户编程界面是怎么样的,这个点我也是有点疑惑的,最近在想 common-bin 如何实现继承覆盖

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

不过这里是否可以改成:

@Injectable({ id: Logger })
class InternalLogger extends Logger {}

@Injectable({ id: Logger })
class ExternalLogger  extends Logger {}

// --

@Inject()
logger: Logger;

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

@atian25 我比较倾向于上面的用法,Inject 靠 design:type 反射,core 里提供基类用作:

  • 继承
  • 容器实例的 id

@atian25
Copy link
Member

atian25 commented Sep 6, 2022

一般我们在底层写基础框架和插件的时候,不会特意去指定 ID 吧?这样会不会有点繁琐,即如果希望被上层覆盖,底层就必须处处写 id ?(我没有倾向性,只是想看看哪个更好)

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

@atian25 关于这一点,core 里不需要指定单独的装饰器只提供基类以供使用,有必要的话可以在上层框架透出类似的装饰器包一层

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

不过这个问题也许可以转换为:

  • 是否要将 Injectable 的语法糖在 core 中就提供
  • 类似 Logger / Trigger 这种 Injectable 语法糖的 id 选择常量 / 基类自身

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

而且这里的 id 指定,只有对应的 Logger 插件开发者或者上层框架开发者才需要感知,应用层的用户就是统一

// 此处的 Logger 是基类
@Inject()
logger:: Logger

所以我认为是否没有必要在 core 中提供冗余的语法糖类装饰器,保持 core 的纯净

@DuanPengfei
Copy link
Collaborator

感觉没回答上天猪的问题??我也有同样的问题,想被覆盖的都得明确提供 id ,这样上层才能使用这个 id 去覆盖吧?

@atian25
Copy link
Member

atian25 commented Sep 6, 2022

我对 ioc 的用法还不是很熟悉。在我那个 bin 的场景下,假设我在 egg-bin 有一个 service,会提供给某个 command 使用。
然后我在 @ali/egg-bin 继承 egg-bin 的情况下,希望把这个 command inject 的 service 换掉,那:

  • 我在 @ali/egg-bin 这一层,如何指定某个新 service 去覆盖这个指定的 service
  • 如果我是期望继承旧的 service,然后再替换掉 command 里面 inject 的它,又应该怎么个写法?

对应的伪代码大概怎么样?

PS:spring 好像是在底层声明 ID?

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

// egg-bin
@Injectable()
export class CommonService {

}

// @ali/egg-bin
@Injectable({ id: CommonService })
export class AliCommonService extends CommonService {

}

这样 egg-bin 中 Inject CommonService 拿到的就是 AliCommonService 实例了

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

用常量覆盖,带来的不便就是既需要关注常量 id,还需要指定类型:

@Inject(COMMON_SERVICE)
service: CommonService

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

昨天看了下 @JerrysShan 在新的 injection 实现中,默认会把 class type 也作为容器 key 写入,这样天然就可以支持 Inject 基类覆盖的方式了

@atian25
Copy link
Member

atian25 commented Sep 6, 2022

// egg-bin
@Injectable()
export class CommonService {

}

// @ali/egg-bin
@Injectable({ id: CommonService })
export class AliCommonService extends CommonService {

}

这样 egg-bin 中 Inject CommonService 拿到的就是 AliCommonService 实例了

嗯,这种写法在继承的情况下可以接受。

另外,如果我是全新写一个 Service 不继承原来的,也需要 import 原来的 Service 来作为 ID 是吧?

@hyj1991
Copy link
Member Author

hyj1991 commented Sep 6, 2022

另外,如果我是全新写一个 Service 不继承原来的,也需要 import 原来的 Service 来作为 ID 是吧?

要看是不是要覆盖,我理解覆盖的场景都是同一个基类,多个不同的实现,并且 Inject 的地方不是自己控制的,这样才会有这种需求。

如果是上面描述的这钟需要覆盖原来实现的,就需要 id

@noahziheng
Copy link
Member

@hyj1991 @atian25
我们这边评估了下,可以先移除 Logger 相关覆盖机制(装饰器/枚举/getter),但我们这边的使用需要一点时间清理掉,完成后我来提 PR 完成移除吧

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 a pull request may close this issue.

4 participants