fix(core): Allow passing AbstractType to the inject function #37958

mitchellwills · 2020-07-07T19:35:07Z

This is a type only change that replaces Type<T>|InjectionToken<T> with
Type<T>|AbstractType<T>|InjectionToken<T> in the injector.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

inject instruction does not accept AbstractType as a token.

Issue Number: N/A

What is the new behavior?

inject instruction accepts AbstractType as a token. This is a type only change (should have no runtime impact)

Does this PR introduce a breaking change?

Yes
No

Other information

Potentially Type<T>|AbstractType<T>|InjectionToken<T> should become a type alias, but i'm not sure what it should be called.

jelbourn

LGTM

Reviewed-for: public-api

petebacondarwin

Reviewed-for: public-api

AndrewKushnir

Hi @mitchellwills, thanks for creating this PR 👍

I did a quick search in Angular codebase and found a couple more places where the AbstractType is not included (both in the current state of the codebase and in your PR):

Could you please have a quick look to see if these places also require type change?

Potentially Type|InjectionToken|AbstractType should become a type alias, but i'm not sure what it should be called.

I really like this idea, could you please create a new ticket (feature request) to capture this proposal?

Thank you.

AndrewKushnir · 2020-07-27T19:45:32Z

@googlebot ping.

atscott

Reviewed-for: public-api

alxhub

Reviewed-for: public-api

LGTM with one suggestion

alxhub · 2020-07-28T17:39:12Z

packages/core/src/di/injector_compatibility.ts

-export function ɵɵinject<T>(token: Type<T>|InjectionToken<T>): T;
-export function ɵɵinject<T>(token: Type<T>|InjectionToken<T>, flags?: InjectFlags): T|null;
-export function ɵɵinject<T>(token: Type<T>|InjectionToken<T>, flags = InjectFlags.Default): T|null {
+export function ɵɵinject<T>(token: Type<T>|InjectionToken<T>|AbstractType<T>): T;


It's just a nitpick, but I would suggest reordering AbstractType and InjectionToken:

Suggested change

export function ɵɵinject<T>(token: Type<T>|InjectionToken<T>|AbstractType<T>): T;

export function ɵɵinject<T>(token: Type<T>|AbstractType<T>|InjectionToken<T>): T;

as they're two sides of the same coin, vs InjectionToken which is more of a foreign currency ;)

atscott

Adding approval for fw-core

petebacondarwin · 2020-11-10T22:08:51Z

@mitchellwills - are you still interested in working on this PR? If so, could you rebase and deal with the comments? Thanks!

mitchellwills · 2020-11-13T03:48:32Z

yah, sorry for the delay (I worked around it so this dropped in priority for me). I'll rebase and address the comments next week

AndrewKushnir · 2020-11-18T01:00:38Z

FYI, related ticket: #23611.

This is a type only change that replaces `Type<T>|InjectionToken<T>` with `Type<T>|AbstractType<T>|InjectionToken<T>` in the injector.

mitchellwills · 2020-11-20T20:47:48Z

I think i've addressed the comments from above: rebase, additional replacements, reorder to AbstractType<T>|InjectionToken<T>.

I also filed #39792

AndrewKushnir · 2020-11-20T21:13:50Z

Presubmit.

AndrewKushnir · 2020-11-24T04:34:22Z

Global Presubmit.

AndrewKushnir · 2020-11-24T18:23:52Z

FYI, global presubmit indicated no problems related to this change, so I'm adding it to the merge queue. Note: I've changed the target from "patch" to "minor" though since this PR can also be considered as a feature (that more types are now allowed in the inject function).

angular-automatic-lock-bot · 2020-12-25T16:43:42Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

pullapprove bot requested review from alxhub, AndrewKushnir, IgorMinar, jelbourn and pkozlowski-opensource July 7, 2020 19:35

jelbourn approved these changes Jul 22, 2020

View reviewed changes

pullapprove bot requested review from petebacondarwin and removed request for IgorMinar July 22, 2020 17:24

petebacondarwin approved these changes Jul 24, 2020

View reviewed changes

AndrewKushnir reviewed Jul 27, 2020

View reviewed changes

AndrewKushnir added cross-cutting: types action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime labels Jul 27, 2020

ngbot bot added this to the needsTriage milestone Jul 27, 2020

AndrewKushnir added the target: patch This PR is targeted for the next patch release label Jul 27, 2020

googlebot added the cla: yes label Jul 27, 2020

pullapprove bot requested review from AndrewKushnir and atscott July 28, 2020 17:03

atscott approved these changes Jul 28, 2020

View reviewed changes

alxhub approved these changes Jul 28, 2020

View reviewed changes

atscott approved these changes Jul 28, 2020

View reviewed changes

petebacondarwin added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 10, 2020

AndrewKushnir mentioned this pull request Nov 18, 2020

fix(core): allow abstract class as injection token #25222

Closed

3 tasks

fix(core): Allow passing AbstractType to the inject function

c80fe19

This is a type only change that replaces `Type<T>|InjectionToken<T>` with `Type<T>|AbstractType<T>|InjectionToken<T>` in the injector.

mitchellwills force-pushed the master branch from 74a1fc5 to c80fe19 Compare November 20, 2020 20:39

mitchellwills mentioned this pull request Nov 20, 2020

There should be a type alias for types that can be used as an injection token #39792

Closed

AndrewKushnir requested a review from mhevery November 20, 2020 20:54

AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Nov 20, 2020

mhevery added the action: merge The PR is ready for merge by the caretaker label Nov 23, 2020

mhevery removed request for pkozlowski-opensource, AndrewKushnir and mhevery November 23, 2020 22:54

AndrewKushnir added target: minor This PR is targeted for the next minor release and removed action: presubmit The PR is in need of a google3 presubmit target: patch This PR is targeted for the next patch release labels Nov 24, 2020

AndrewKushnir closed this in a1b6ad0 Nov 24, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 25, 2020

pullapprove bot added the comp: docs/api label Dec 25, 2020

ngbot bot modified the milestones: needsTriage, Backlog Dec 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): Allow passing AbstractType to the inject function #37958

fix(core): Allow passing AbstractType to the inject function #37958

mitchellwills commented Jul 7, 2020 •

edited

jelbourn left a comment

petebacondarwin left a comment

AndrewKushnir left a comment

AndrewKushnir commented Jul 27, 2020

atscott left a comment

alxhub left a comment

alxhub Jul 28, 2020

atscott left a comment

petebacondarwin commented Nov 10, 2020

mitchellwills commented Nov 13, 2020

AndrewKushnir commented Nov 18, 2020

mitchellwills commented Nov 20, 2020

AndrewKushnir commented Nov 20, 2020

AndrewKushnir commented Nov 24, 2020

AndrewKushnir commented Nov 24, 2020

angular-automatic-lock-bot bot commented Dec 25, 2020

	export function ɵɵinject<T>(token: Type<T>\|InjectionToken<T>\|AbstractType<T>): T;
	export function ɵɵinject<T>(token: Type<T>\|AbstractType<T>\|InjectionToken<T>): T;

fix(core): Allow passing AbstractType to the inject function #37958

fix(core): Allow passing AbstractType to the inject function #37958

Conversation

mitchellwills commented Jul 7, 2020 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

jelbourn left a comment

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

AndrewKushnir commented Jul 27, 2020

atscott left a comment

Choose a reason for hiding this comment

alxhub left a comment

Choose a reason for hiding this comment

alxhub Jul 28, 2020

Choose a reason for hiding this comment

atscott left a comment

Choose a reason for hiding this comment

petebacondarwin commented Nov 10, 2020

mitchellwills commented Nov 13, 2020

AndrewKushnir commented Nov 18, 2020

mitchellwills commented Nov 20, 2020

AndrewKushnir commented Nov 20, 2020

AndrewKushnir commented Nov 24, 2020

AndrewKushnir commented Nov 24, 2020

angular-automatic-lock-bot bot commented Dec 25, 2020

mitchellwills commented Jul 7, 2020 •

edited