refactor(core): add single type for injector token by crisbeto · Pull Request #41580 · angular/angular

crisbeto · 2021-04-12T15:23:19Z

Currently we have a lot of places where we repeat the type Type<T>|AbstractType<T>|InjectionToken<T> which makes it cumbersome to add another type or to type something else with the same signature.

These changes add a new type that can be used instead.

Fixes #39792.

mary-poppins · 2021-04-12T15:42:39Z

You can preview 2daf941 at https://pr41580-2daf941.ngbuilds.io/.

petebacondarwin · 2021-04-12T16:04:01Z

To be honest, I am not convinced this is a valuable change.

crisbeto · 2021-04-12T16:16:20Z

IMO it's worth having, because it's a safe change that will allow us to avoid having to adjust 20+ places if we have to make a change similar to #37958.

crisbeto · 2021-04-19T19:43:16Z

Renamed to ProviderToken as discussed.

mary-poppins · 2021-04-19T20:30:19Z

You can preview 7fcc8b9 at https://pr41580-7fcc8b9.ngbuilds.io/.

mary-poppins · 2021-04-19T21:05:53Z

You can preview 56d8661 at https://pr41580-56d8661.ngbuilds.io/.

Currently we have a lot of places where we repeat the type `Type<T>|AbstractType<T>|InjectionToken<T>` which makes it cumbersome to add another type or to type something else with the same signature. These changes add a new type that can be used instead. Fixes angular#39792.

mary-poppins · 2021-04-20T18:19:41Z

You can preview 38d7c4d at https://pr41580-38d7c4d.ngbuilds.io/.

petebacondarwin

NIce! LGTM

mary-poppins · 2021-04-20T22:49:11Z

You can preview 2daf941 at https://pr41580-2daf941.ngbuilds.io/.
You can preview 7fcc8b9 at https://pr41580-7fcc8b9.ngbuilds.io/.
You can preview 56d8661 at https://pr41580-56d8661.ngbuilds.io/.
You can preview 38d7c4d at https://pr41580-38d7c4d.ngbuilds.io/.

AndrewKushnir

LGTM 👍

I feel like we may need to expand docs on the ProviderToken symbol (for ex. to describe its practical use for developers), but we can do that as a followup (may be create a ticket on that?).

AndrewKushnir · 2021-04-20T22:55:05Z

Presubmit.

AndrewKushnir

Reviewed-for: public-api

(note: I've removed the "merge" label as more people from "public-api" group should review the changes first)

petebacondarwin

Reviewed-for: public-api

jelbourn

LGTM

Reviewed-for: public-api

AndrewKushnir · 2021-04-21T20:34:30Z

@crisbeto FYI the current target label is "target: minor", so the change will land only in master (which targets first minor after v12, i.e. v12.1.0). I guess we may want to include that into v12 too? (note to caretaker: this comment is not a merge-blocker, we can merge it with the current set of labels and create a followup PR if needed)

crisbeto · 2021-04-21T21:02:04Z

Not sure why it was targeting minor. Switching it to rc.

Currently we have a lot of places where we repeat the type `Type<T>|AbstractType<T>|InjectionToken<T>` which makes it cumbersome to add another type or to type something else with the same signature. These changes add a new type that can be used instead. Fixes #39792. PR Close #41580

angular-automatic-lock-bot · 2021-05-23T16:08:25Z

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.}

google-cla Bot added the cla: yes label Apr 12, 2021

crisbeto commented Apr 12, 2021

View reviewed changes

Comment thread packages/core/src/di/token.ts Resolved

crisbeto force-pushed the 39792/di-token branch from 6f718e1 to 2daf941 Compare April 12, 2021 15:32

petebacondarwin added aio: preview target: minor This PR is targeted for the next minor release labels Apr 12, 2021

crisbeto marked this pull request as ready for review April 12, 2021 15:55

pullapprove Bot requested review from AndrewKushnir and alxhub April 12, 2021 15:56

crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime labels Apr 12, 2021

ngbot Bot modified the milestone: Backlog Apr 12, 2021

AndrewKushnir added the action: discuss label Apr 13, 2021

crisbeto removed the action: discuss label Apr 19, 2021

crisbeto force-pushed the 39792/di-token branch from 8065be8 to 7fcc8b9 Compare April 19, 2021 20:16

crisbeto force-pushed the 39792/di-token branch from 7fcc8b9 to 56d8661 Compare April 19, 2021 20:52

petebacondarwin suggested changes Apr 20, 2021

View reviewed changes

crisbeto added 3 commits April 20, 2021 20:01

fixup! refactor(core): add single type for injector token

9f0b22e

fixup! refactor(core): add single type for injector token

38d7c4d

crisbeto force-pushed the 39792/di-token branch from 56d8661 to 38d7c4d Compare April 20, 2021 18:10

petebacondarwin approved these changes Apr 20, 2021

View reviewed changes

crisbeto removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 20, 2021

crisbeto added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed aio: preview labels Apr 20, 2021

AndrewKushnir added the aio: preview label Apr 20, 2021

AndrewKushnir removed the aio: preview label Apr 20, 2021

AndrewKushnir approved these changes Apr 20, 2021

View reviewed changes

pullapprove Bot requested review from AndrewKushnir, IgorMinar and jelbourn April 20, 2021 22:54

AndrewKushnir removed the action: merge The PR is ready for merge by the caretaker label Apr 20, 2021

AndrewKushnir approved these changes Apr 20, 2021

View reviewed changes

pullapprove Bot requested a review from atscott April 20, 2021 22:56

petebacondarwin approved these changes Apr 21, 2021

View reviewed changes

jelbourn approved these changes Apr 21, 2021

View reviewed changes

crisbeto added the action: merge The PR is ready for merge by the caretaker label Apr 21, 2021

AndrewKushnir removed request for IgorMinar, alxhub and atscott April 21, 2021 20:30

AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Apr 21, 2021

crisbeto added target: rc This PR is targeted for the next release-candidate and removed target: minor This PR is targeted for the next minor release labels Apr 21, 2021

jessicajaniuk closed this in de4fafb Apr 22, 2021

angular-automatic-lock-bot Bot locked and limited conversation to collaborators May 23, 2021

Conversation

crisbeto commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mary-poppins commented Apr 12, 2021

Uh oh!

petebacondarwin commented Apr 12, 2021

Uh oh!

crisbeto commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crisbeto commented Apr 19, 2021

Uh oh!

mary-poppins commented Apr 19, 2021

Uh oh!

mary-poppins commented Apr 19, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mary-poppins commented Apr 20, 2021

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Apr 20, 2021

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Apr 20, 2021

Uh oh!

AndrewKushnir left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Apr 21, 2021

Uh oh!

crisbeto commented Apr 21, 2021

Uh oh!

angular-automatic-lock-bot Bot commented May 23, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

crisbeto commented Apr 12, 2021 •

edited

Loading

crisbeto commented Apr 12, 2021 •

edited

Loading

AndrewKushnir left a comment •

edited

Loading