-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
fix(core): take @Host into account while processing useFactory
arguments
#40122
Conversation
d6b851b
to
0f361bc
Compare
@@ -3,7 +3,7 @@ | |||
"master": { | |||
"uncompressed": { | |||
"runtime-es2015": 1485, | |||
"main-es2015": 140333, | |||
"main-es2015": 140871, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewers: this is very likely to be an accumulated increase. This PR does introduce a minor increase as well by making Host
decorator non-tree-shakable, but the cost of having it very small, since the logic to create this decorator is shared among all DI decorators (which are already referred in the same code). In the future we can make all these decorators tree-shakable, see #40143.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In cases like this, it might even be worth adding a separate commit before this one, which simply updates the value to the "current" size. Then in the commit that makes the fix, it will show the delta accurately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - nice catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed-for: size-tracking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
reviewed-for: fw-core
reviewed-for: size-tracking
…ments DI providers can be defined via `useFactory` function, which may have arguments configured via `deps` array. The `deps` array may contain DI flags represented by DI decorators (such as `@Self`, `@SkipSelf`, etc). Prior to this commit, having the `@Host` decorator in `deps` array resulted in runtime error in Ivy. The problem was that the `@Host` decorator was not taken into account while `useFactory` argument list was constructed, the `@Host` decorator was treated as a token that should be looked up. This commit updates the logic which prepares `useFactory` arguments to recognize the `@Host` decorator.
0f361bc
to
186e469
Compare
Moving to target minor only per offline discussion, @AndrewKushnir will create a patch-only follow up of the same change. |
…ments (angular#40122) DI providers can be defined via `useFactory` function, which may have arguments configured via `deps` array. The `deps` array may contain DI flags represented by DI decorators (such as `@Self`, `@SkipSelf`, etc). Prior to this commit, having the `@Host` decorator in `deps` array resulted in runtime error in Ivy. The problem was that the `@Host` decorator was not taken into account while `useFactory` argument list was constructed, the `@Host` decorator was treated as a token that should be looked up. This commit updates the logic which prepares `useFactory` arguments to recognize the `@Host` decorator. PR Close angular#40122
…ments (#40122) (#40313) DI providers can be defined via `useFactory` function, which may have arguments configured via `deps` array. The `deps` array may contain DI flags represented by DI decorators (such as `@Self`, `@SkipSelf`, etc). Prior to this commit, having the `@Host` decorator in `deps` array resulted in runtime error in Ivy. The problem was that the `@Host` decorator was not taken into account while `useFactory` argument list was constructed, the `@Host` decorator was treated as a token that should be looked up. This commit updates the logic which prepares `useFactory` arguments to recognize the `@Host` decorator. PR Close #40122 PR Close #40313
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
DI provider can be defined via
useFactory
function, which may have arguments configured viadeps
array.The
deps
array may contain DI flags represented by DI decorators (such as@Self
,@SkipSelf
, etc). Prior to thiscommit, having the
@Host
decorator indeps
array resulted in runtime error in Ivy. The problem was that the@Host
decorator was not taken into account while
useFactory
argument list was constructed, the@Host
decorator wastreated as a token that should be looked up.
This commit updates the logic which prepares
useFactory
arguments to recognize the@Host
decorator.PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?