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

feature(injector): PartialFactory #487

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Conversation

timvandam
Copy link
Contributor

Summary of changes

Utility PartialFactory type that makes it easy to resolve providers when instantiating non-provider classes.

class A {
    constructor (public readonly b: B, public readonly num: number) {
    }
}

class B {
    public b = 'b';
}

const injector = Injector.from([B]);
const factory = injector.get<PartialFactory<A>>();

const a = factory({
    num: 5,
});
// A { B { b: 'b' }, num: 5 }

Relinquishment of Rights

Please mark following checkbox to confirm that you relinquish all rights of your changes:

  • I waive and relinquish all rights regarding this changes (including code, text, and images) to Deepkit UG (limited), Germany. This changes (including code, text, and images) are under MIT license without name attribution, copyright notice, and permission notice requirement.

@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2023

Codecov Report

Merging #487 (99720e2) into master (a47111c) will increase coverage by 0.12%.
The diff coverage is 92.59%.

❗ Current head 99720e2 differs from pull request most recent head 3bb7984. Consider uploading reports for the commit 3bb7984 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
+ Coverage   78.44%   78.57%   +0.12%     
==========================================
  Files         175      175              
  Lines       18644    18695      +51     
  Branches     4867     4882      +15     
==========================================
+ Hits        14625    14689      +64     
+ Misses       4019     4006      -13     
Files Coverage Δ
packages/injector/src/injector.ts 84.11% <92.59%> (+2.70%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@timvandam timvandam force-pushed the partial-factory branch 6 times, most recently from d9d978e to c9ff318 Compare October 2, 2023 12:49
@@ -808,7 +837,8 @@ export class Injector implements InjectorInterface {
}
current = current.indexAccessOrigin.container;
}
return () => config;

if (config) return () => config;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to change this to support creating factories for interfaces without having to Inject<.> every property. Interface properties always have a property.type.indexAccessOrigin set, which triggers config lookup. Without this change it would always return undefined (which was undesirable behavior in the first place I think)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah this broke some stuff. Fixed in c4ed83a

@marcj marcj merged commit c730aed into deepkit:master Oct 2, 2023
5 checks passed
@marcj
Copy link
Member

marcj commented Oct 2, 2023

awesome, thanks!

marcj added a commit that referenced this pull request Oct 3, 2023
This pull request was closed.
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.

3 participants