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

allow parametrized resolution of services through getParametrized method #79

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DGrudzynskyi
Copy link

@DGrudzynskyi DGrudzynskyi commented Feb 9, 2021

Hi Thiago,

Thank you for working on typescript-ioc.
The package is great! To me it looks like the "proper" implementation of IoC container, with regards that information about interfaces is not propagated to runtime.

This PR is a suggestion about supporting the design pattern I am using a lot while working in MVVM approach. It comes from classic WPF's MVVM but the same looks handy while working with typescript UI using MVVM-ish architecture.

The UI is commonly represented with components(controls) tree. In MVVM approach, component is driven by the data, gathered and persisted within the viewmodel, dedicated to this component or to components subtree.

ViewModels essentially have a dependencies on the domain/infrastructural services. This is where IoC container comes to play.
However, viewmodel might by instantiated within the components subtree so it might rely on the instance-specific parameters.

Consider an example: we have a list of accounts with a complex and comprehensive UI.
We may want to build a viewmodel-per-account which would have the following dependencies: accountId, accountsDao.
AccountsDao can be injected by IoC container, however accountId is specific to particular viewmodel instance.

Below is an ugly solution I am using at the moment for my actual projects:

class AccountVM {
    constructor(accountId: number, accountDao: IAccountsDao = IoC.Container.get(IAccountsDao)) {}
}

But this solution prevents from using single resolution context while calling new AccountVM(accountId). Scope.Request is effectively bypassed while working with this approach.

This PR contains the new public function
getParametrized<T>(source: Function & { prototype: T }, ...contextParams: Array<any>)

For the example above, it is supposed to be called this way:

Container.getParametrized(AccountVM, 1)

The function replaces first X arguments of constructor with the X values, provided as rest parameters of Container.getParametrized function.

I've added some test coverage to illustrate the usage and the way instance-specific parameters interferes with injected parameters.

Note, that I've also modified the way @Inject decorator for constructor arguments works.
Previously, if developer does this:

class Dddd {
        constructor(@Inject a: Aaaa, b: Bbbb, @Inject c: Cccc) {
        }
    }

Container.get(Dddd)

instance of type Cccc, would have been injected in the constructor argument having 1st position. So in runtime, one would have variable b of type Cccc. Even though such usage of decorators is highly questionable, it still doesn't looks as a correct behavior.

Same change is applied to @InjectValue decorator.

Let me know your thoughts about this PR.
I know that adding new methods to the public API should be done with a huge caution so consider this PR as a starting point for conversation, rather than an actual pull request.

@codecov
Copy link

codecov bot commented Feb 9, 2021

Codecov Report

Merging #79 (aaf0f8c) into master (e0287c9) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
+ Coverage   98.58%   98.64%   +0.05%     
==========================================
  Files           8        8              
  Lines         425      442      +17     
  Branches       71       75       +4     
==========================================
+ Hits          419      436      +17     
  Misses          6        6              
Impacted Files Coverage Δ
src/container/container-binding-config.ts 100.00% <100.00%> (ø)
src/decorators.ts 97.87% <100.00%> (ø)
src/model.ts 100.00% <100.00%> (ø)
src/typescript-ioc.ts 100.00% <100.00%> (ø)

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 e0287c9...aaf0f8c. Read the comment docs.

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.

None yet

1 participant