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(core): remove TestBed.deprecatedOverrideProvider #30576

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@kara
Copy link
Contributor

commented May 20, 2019

BREAKING CHANGE

In PR #19558, we fixed a bug in TestBed.overrideProvider where
eager providers were not being instantiated correctly. However,
it turned out that since this bug had been around for quite a bit,
many apps were relying on the broken behavior where the providers
would not be instantiated. To assist in the transition, the
TestBed.deprecatedOverrideProvider method was temporarily
introduced to mimic the old behavior so that apps would have a
longer time period to migrate their code.

2 years and 3 versions later, it is time to remove the temporary
method. This commit removes TestBed.deprecatedOverrideProvider
altogether. Any usages of TestBed.deprecatedOverrideProvider
should be replaced with TestBed.overrideProvider. This may mean
that providers that were not created before will now be instantiated,
which could mean that your tests need to provide more mocks or stubs
for the dependencies of the newly instantiated providers.

@googlebot googlebot added the cla: yes label May 20, 2019

@kara kara force-pushed the kara:deprecatedOverrideProvider branch 2 times, most recently from 6f18717 to 8d7ffe0 May 20, 2019

@kara kara added the aio: preview label May 20, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented May 20, 2019

@kara

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

@mary-poppins

This comment has been minimized.

Copy link

commented May 21, 2019

@kara kara removed the aio: preview label May 21, 2019

@kara kara force-pushed the kara:deprecatedOverrideProvider branch from 8d7ffe0 to 70cd7ad May 21, 2019

@kara kara changed the title WIP: refactor(core): remove TestBed.deprecatedOverrideProvider refactor(core): remove TestBed.deprecatedOverrideProvider May 21, 2019

@ngbot ngbot bot added this to the needsTriage milestone May 21, 2019

@kara

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

@kara kara marked this pull request as ready for review May 21, 2019

@kara kara requested review from IgorMinar and angular/fw-core as code owners May 21, 2019

@alfaproject

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

Refactoring means functionality and especially API surface doesn't change. What you are doing is a feature. Something is being added or removed in this case. (;

@IgorMinar
Copy link
Member

left a comment

lgtm.

confidence note: we've done research in google3 and found no references and while github has over a thousand references they all refer to vendored angular d.ts files. We don't think that anyone is really using these apis and they just take up space and confuse people.

@IgorMinar

This comment has been minimized.

Copy link
Member

commented May 21, 2019

I updated #30400 to reflect this change

@IgorMinar IgorMinar modified the milestones: needsTriage, version 8 May 21, 2019

@kara

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

merge-assistance: Igor has global approval and I'm an owner of core

@kara

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

@alfaproject Yeah, "refactor" doesn't seem quite right, but neither does "feature" (is it really a feature to remove something?). I think it's probably fine for the circumstances, but maybe "fix" would be better? 🤔

Edit: updated to "fix"

fix(core): remove deprecated `TestBed.deprecatedOverrideProvider` API
BREAKING CHANGE

In PR #19558, we fixed a bug in `TestBed.overrideProvider` where
eager providers were not being instantiated correctly. However,
it turned out that since this bug had been around for quite a bit,
many apps were relying on the broken behavior where the providers
would not be instantiated. To assist in the transition, the
`TestBed.deprecatedOverrideProvider` method was temporarily
introduced to mimic the old behavior so that apps would have a
longer time period to migrate their code.

2 years and 3 versions later, it is time to remove the temporary
method. This commit removes `TestBed.deprecatedOverrideProvider`
altogether. Any usages of `TestBed.deprecatedOverrideProvider`
should be replaced with `TestBed.overrideProvider`. This may mean
that providers that were not created before will now be instantiated,
which could mean that your tests need to provide more mocks or stubs
for the dependencies of the newly instantiated providers.

@kara kara force-pushed the kara:deprecatedOverrideProvider branch from 70cd7ad to 28dd132 May 21, 2019

@kara kara changed the title refactor(core): remove TestBed.deprecatedOverrideProvider fix(core): remove TestBed.deprecatedOverrideProvider May 21, 2019

@jasonaden jasonaden closed this in a96976e May 21, 2019

jasonaden added a commit that referenced this pull request May 21, 2019

fix(core): remove deprecated `TestBed.deprecatedOverrideProvider` API (
…#30576)

BREAKING CHANGE

In PR #19558, we fixed a bug in `TestBed.overrideProvider` where
eager providers were not being instantiated correctly. However,
it turned out that since this bug had been around for quite a bit,
many apps were relying on the broken behavior where the providers
would not be instantiated. To assist in the transition, the
`TestBed.deprecatedOverrideProvider` method was temporarily
introduced to mimic the old behavior so that apps would have a
longer time period to migrate their code.

2 years and 3 versions later, it is time to remove the temporary
method. This commit removes `TestBed.deprecatedOverrideProvider`
altogether. Any usages of `TestBed.deprecatedOverrideProvider`
should be replaced with `TestBed.overrideProvider`. This may mean
that providers that were not created before will now be instantiated,
which could mean that your tests need to provide more mocks or stubs
for the dependencies of the newly instantiated providers.

PR Close #30576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.