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(common): temporarily re-export and deprecate XhrFactory #41393

Closed

Conversation

petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented Mar 31, 2021

Docs show deprecation notice: https://pr41393-f625e20.ngbuilds.io/api/common/http/XhrFactory
Tested against ngcc-validation: angular/ngcc-validation#2820
Deprecation message also shows in IDE:
Screenshot 2021-03-31 at 17 12 25

@mary-poppins
Copy link

You can preview 454f9a3 at https://pr41393-454f9a3.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 60f4504 at https://pr41393-60f4504.ngbuilds.io/.

@petebacondarwin petebacondarwin force-pushed the xhr-factory-reexport branch 2 times, most recently from 15d17bd to 2509b87 Compare March 31, 2021 11:48
@mary-poppins
Copy link

You can preview 2509b87 at https://pr41393-2509b87.ngbuilds.io/.

@mary-poppins
Copy link

You can preview afa3d38 at https://pr41393-afa3d38.ngbuilds.io/.

The moved `XhrFactory` still needs to be available from `@angular/common/http`
for some libraries that were built prior to 12.0.0, otherwise they cannot be
used in applications built post-12.0.0.

This commit adds back the re-export of `XhrFactory` and deprecates it.
@petebacondarwin petebacondarwin changed the title WIP - NOTHING TO SEE HERE :-) fix(common): temporarily re-export and deprecate XhrFactory Mar 31, 2021
@petebacondarwin petebacondarwin marked this pull request as ready for review March 31, 2021 15:44
@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Mar 31, 2021
@pullapprove pullapprove bot requested review from atscott and crisbeto March 31, 2021 15:46
@mary-poppins
Copy link

You can preview f625e20 at https://pr41393-f625e20.ngbuilds.io/.

Copy link
Contributor

@atscott atscott 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 atscott March 31, 2021 16:14
@mary-poppins
Copy link

You can preview f69734e at https://pr41393-f69734e.ngbuilds.io/.

Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM, not sure why this didn't work for me when I tried it. Clearly, I was doing something different.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

👍

Shouldn't we also add a note in aio/content/guide/deprecations.md?

@gkalpak gkalpak added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Mar 31, 2021
We now include `misc/angular-in-memory-web-api` in the packages that
are build in the `dist/packages-dist` directory. But this package should
not be published as an artifact.

This commit prevents attempting to publishing the `misc` directory by
first checking whether the directory contains a `package.json` file.
@mary-poppins
Copy link

You can preview 3e30358 at https://pr41393-3e30358.ngbuilds.io/.

Copy link
Contributor

@AndrewKushnir AndrewKushnir 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

@AndrewKushnir
Copy link
Contributor

Presubmit.

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: docs-packaging-and-releasing

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

@AndrewKushnir AndrewKushnir added area: common Issues related to APIs in the @angular/common package and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 1, 2021
@ngbot ngbot bot added this to the Backlog milestone Apr 1, 2021
@AndrewKushnir
Copy link
Contributor

@petebacondarwin FYI presubmits are successful for the changes in this PR.

@AndrewKushnir AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Apr 1, 2021
@petebacondarwin petebacondarwin added the action: merge The PR is ready for merge by the caretaker label Apr 1, 2021
@alxhub alxhub added target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Apr 1, 2021
@alxhub alxhub closed this in 7dfa446 Apr 1, 2021
alxhub pushed a commit that referenced this pull request Apr 1, 2021
We now include `misc/angular-in-memory-web-api` in the packages that
are build in the `dist/packages-dist` directory. But this package should
not be published as an artifact.

This commit prevents attempting to publishing the `misc` directory by
first checking whether the directory contains a `package.json` file.

PR Close #41393
@petebacondarwin petebacondarwin deleted the xhr-factory-reexport branch April 1, 2021 19:08
TeriGlover pushed a commit to TeriGlover/angular that referenced this pull request Apr 5, 2021
…r#41393)

The moved `XhrFactory` still needs to be available from `@angular/common/http`
for some libraries that were built prior to 12.0.0, otherwise they cannot be
used in applications built post-12.0.0.

This commit adds back the re-export of `XhrFactory` and deprecates it.

PR Close angular#41393
TeriGlover pushed a commit to TeriGlover/angular that referenced this pull request Apr 5, 2021
…41393)

We now include `misc/angular-in-memory-web-api` in the packages that
are build in the `dist/packages-dist` directory. But this package should
not be published as an artifact.

This commit prevents attempting to publishing the `misc` directory by
first checking whether the directory contains a `package.json` file.

PR Close angular#41393
@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 May 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker aio: preview area: common/http area: common Issues related to APIs in the @angular/common package cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants