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

fix(ivy): provider override via TestBed should remove old providers from the list #33706

Closed

Conversation

@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Nov 9, 2019

While overriding providers in Ivy TestBed (via TestBed.overrideProvider call), the old providers were retained in the list, since the override takes precedence. However, presence of providers in the list might have side-effect: if a provider has the ngOnDestroy lifecycle hook, this hook will be registered and invoked later (when component is destroyed). This commit updates TestBed logic to clear provider list by removing the ones which have overrides.

This PR resolves FW-1670.

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No
@AndrewKushnir

This comment has been minimized.

Copy link
Contributor Author

AndrewKushnir commented Nov 9, 2019

Initial set of g3 presubmits:

@AndrewKushnir AndrewKushnir force-pushed the AndrewKushnir:ngondestroy_for_providers branch from 2d5dae7 to 8698dbe Nov 9, 2019
…rom the list

While overriding providers in Ivy TestBed (via TestBed.overrideProvider call), the old providers were retained in the list, since the override takes precedence. However, presence of providers in the list might have side-effect: if a provider has the `ngOnDestroy` lifecycle hook, this hook will be registered and invoked later (when component is destroyed). This commit updates TestBed logic to clear provider list by removing the ones which have overrides.
@AndrewKushnir AndrewKushnir force-pushed the AndrewKushnir:ngondestroy_for_providers branch from 453bf71 to 0564667 Nov 9, 2019
@AndrewKushnir AndrewKushnir marked this pull request as ready for review Nov 9, 2019
@AndrewKushnir AndrewKushnir requested a review from angular/fw-core as a code owner Nov 9, 2019
@AndrewKushnir AndrewKushnir requested a review from atscott Nov 9, 2019
Copy link
Member

pkozlowski-opensource left a comment

LGTM

@AndrewKushnir

This comment has been minimized.

Copy link
Contributor Author

AndrewKushnir commented Nov 11, 2019

FYI, presubmits (including global for Ivy) look good. Thank you.

kara added a commit that referenced this pull request Nov 11, 2019
…rom the list (#33706)

While overriding providers in Ivy TestBed (via TestBed.overrideProvider call), the old providers were retained in the list, since the override takes precedence. However, presence of providers in the list might have side-effect: if a provider has the `ngOnDestroy` lifecycle hook, this hook will be registered and invoked later (when component is destroyed). This commit updates TestBed logic to clear provider list by removing the ones which have overrides.

PR Close #33706
@kara kara closed this in eece138 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.