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.

Sign up for GitHub

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

Jump to bottom

feat(core): update Injector.get and TestBed.inject to support object-based flags #46761

Closed
wants to merge 3 commits into from
Closed

feat(core): update Injector.get and TestBed.inject to support object-based flags #46761

wants to merge 3 commits into from

Conversation

AndrewKushnir
Copy link
Contributor

@AndrewKushnir AndrewKushnir commented Jul 9, 2022 •
edited

See individual commits.

PR Type

What kind of change does this PR introduce?

  • Feature

Does this PR introduce a breaking change?

  • Yes
  • No

@AndrewKushnir AndrewKushnir added feature Issue that requests a new feature action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release flag: deprecation labels Jul 9, 2022
@AndrewKushnir AndrewKushnir added target: major This PR is targeted for the next major release breaking changes and removed target: minor This PR is targeted for the next minor release labels Jul 9, 2022
@AndrewKushnir AndrewKushnir force-pushed the injector_flags_as_obj branch 2 times, most recently from 3553e1e to 01faada Compare July 10, 2022 20:55
@AndrewKushnir
Copy link
Contributor Author

AndrewKushnir commented Jul 10, 2022 •
edited

Exploratory Presubmit + TGP.

@AndrewKushnir AndrewKushnir added target: minor This PR is targeted for the next minor release and removed target: major This PR is targeted for the next major release breaking changes labels Jul 10, 2022
@jessicajaniuk jessicajaniuk added the area: core Issues related to the framework runtime label Jul 11, 2022
@ngbot ngbot bot added this to the Backlog milestone Jul 11, 2022
atscott
atscott requested changes Jul 12, 2022
View reviewed changes
goldens/public-api/core/testing/index.md Outdated Show resolved Hide resolved
@AndrewKushnir
Copy link
Contributor Author

TGP #2.

alxhub
alxhub reviewed Jul 13, 2022
View reviewed changes
Copy link
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

FYI, I think that modifying the signature of Injector.get is a breaking change. We should add the appropriate notice and push this to v15.

packages/core/src/di/injector_compatibility.ts Outdated Show resolved Hide resolved
packages/core/src/di/injector_compatibility.ts Show resolved Hide resolved
@AndrewKushnir AndrewKushnir added breaking changes target: major This PR is targeted for the next major release state: blocked and removed target: minor This PR is targeted for the next minor release action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 13, 2022
@AndrewKushnir
feat(core): support object-based DI flags in Injector.get()
7482814 
This commit applies the changes similar to the ones performed for the `inject()` function in angular@df246bb.

The `Injector.get` function is updated to use previously added object-based API for options: now the flags argument supports passing an object which configures injection flags.

DEPRECATED:

The bit field signature of `Injector.get()` has been deprecated, in favor of the new options object.
@AndrewKushnir
feat(core): support object-based DI flags in TestBed.inject()
ad411b8 
This commit applies the changes similar to the ones performed for the `inject()` function in angular@df246bb.

The `TestBed.inject` function is updated to use previously added object-based API for options: now the flags argument supports passing an object which configures injection flags.

DEPRECATED:

The bit field signature of `TestBed.inject()` has been deprecated, in favor of the new options object.
@AndrewKushnir
Copy link
Contributor Author

@alxhub I've updated the API and added a couple tests as we discussed (fixup commit: 6159102). Could you please take another look?

@AndrewKushnir AndrewKushnir removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 24, 2022
@AndrewKushnir
Copy link
Contributor Author

TGP after most recent updates.

AndrewKushnir added a commit to AndrewKushnir/ngx-toastr that referenced this pull request Sep 26, 2022
@AndrewKushnir
refactor: remove ToastInjector in favor of creating an instance via I…
9d16233 
…njector.create

This commit removes a custom ToastInjector (that provided a single token) in favor of creating a new injector instance using the `Injector.create` call.

The goal of this change is to make the library forward-compatible with an upcoming change to the Injector interface, see angular/angular#46761.
@AndrewKushnir AndrewKushnir mentioned this pull request Sep 26, 2022
Merged
@AndrewKushnir
Copy link
Contributor Author

One more TGP before the submission.

jessicajaniuk
jessicajaniuk approved these changes Sep 27, 2022
View reviewed changes
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@pullapprove pullapprove bot requested a review from alxhub September 27, 2022 15:44
alxhub
alxhub approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

jelbourn
jelbourn approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: public-api

pkozlowski-opensource
pkozlowski-opensource approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member

@pkozlowski-opensource pkozlowski-opensource left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: public-api
Reviewed-for: fw-core

@alxhub alxhub removed the request for review from atscott September 27, 2022 17:04
@alxhub alxhub added the action: merge The PR is ready for merge by the caretaker label Sep 27, 2022
@alxhub
Copy link
Member

alxhub commented Sep 27, 2022

This PR was merged into the repository by commit 120555a.

@alxhub alxhub closed this in 841c8e5 Sep 27, 2022
alxhub pushed a commit that referenced this pull request Sep 27, 2022
@AndrewKushnir @alxhub
feat(core): support object-based DI flags in TestBed.inject() (#46761)
120555a 
This commit applies the changes similar to the ones performed for the `inject()` function in df246bb.

The `TestBed.inject` function is updated to use previously added object-based API for options: now the flags argument supports passing an object which configures injection flags.

DEPRECATED:

The bit field signature of `TestBed.inject()` has been deprecated, in favor of the new options object.

PR Close #46761
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@atscott atscott atscott approved these changes

@jelbourn jelbourn jelbourn approved these changes

@pkozlowski-opensource pkozlowski-opensource pkozlowski-opensource approved these changes

@alxhub alxhub alxhub approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Assignees

@AndrewKushnir AndrewKushnir

Labels
action: global presubmit The PR is in need of a google3 global presubmit action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime breaking changes feature Issue that requests a new feature target: major This PR is targeted for the next major release
Projects
None yet
Milestone
v15 candidates
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@AndrewKushnir @alxhub @atscott @jelbourn @pkozlowski-opensource @jessicajaniuk @dylhunn