Skip to content

fix(http): allow to combine FetchBackend and InMemory services in the providers #52425

Closed
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:fix/in-memory-with-fetch
Closed

fix(http): allow to combine FetchBackend and InMemory services in the providers #52425
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:fix/in-memory-with-fetch

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche commented Oct 27, 2023 •
edited
Loading

When using withFetch, the PRIMARY_HTTP_BACKEND token is set.

The InMemory Backend services will also set that token.

This means that providers order will matter and the latest on the list will be the one instantiated.

cc @alan-agius4, this is related to your comment on #52398

@JeanMeche JeanMeche force-pushed the fix/in-memory-with-fetch branch 3 times, most recently from 1c20160 to de13627 Compare October 27, 2023 18:20
@JeanMeche JeanMeche changed the title fix(http): Don't override the backend when using the InMemoryWebAPI fix(http): allow to combine FetchBackend and InMemory services in the providers Oct 27, 2023
@JeanMeche JeanMeche marked this pull request as ready for review October 27, 2023 18:22
@pullapprove pullapprove bot requested a review from jessicajaniuk October 27, 2023 18:22
@JeanMeche JeanMeche force-pushed the fix/in-memory-with-fetch branch from de13627 to 740f196 Compare October 27, 2023 18:26
@JeanMeche JeanMeche added target: rc This PR is targeted for the next release-candidate action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 27, 2023
jessicajaniuk
jessicajaniuk approved these changes Oct 27, 2023
View reviewed changes
Copy link
Copy Markdown
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.

LGTM!

@jessicajaniuk jessicajaniuk added the area: common/http Issues related to HTTP and HTTP Client label Oct 27, 2023
@ngbot ngbot bot added this to the Backlog milestone Oct 27, 2023
@jessicajaniuk jessicajaniuk added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 27, 2023
@dylhunn dylhunn added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Oct 31, 2023
@dylhunn
Copy link
Copy Markdown
Contributor

dylhunn commented Oct 31, 2023

Caretaker -- the aio failure looks spurious to me

@alxhub
Copy link
Copy Markdown
Member

alxhub commented Oct 31, 2023

This PR was merged into the repository by commit 49b037f.

alxhub pushed a commit that referenced this pull request Oct 31, 2023
@JeanMeche @alxhub
fix(http): Don't override the backend when using the InMemoryWebAPI (#…
8156b3d 
…52425)

When using `withFetch`,  the `PRIMARY_HTTP_BACKEND` token is set.

The InMemory Backend services will also set that token.

This means that providers order will matter and the latest on the list will be the one instantiated

PR Close #52425
@alxhub alxhub closed this in 49b037f Oct 31, 2023
@JeanMeche JeanMeche deleted the fix/in-memory-with-fetch branch October 31, 2023 20:17
alxhub added a commit that referenced this pull request Oct 31, 2023
@alxhub
Revert "fix(http): Don't override the backend when using the InMemory…
de802f0 
…WebAPI (#52425)"

This reverts commit 49b037f. Reason: it
breaks tests in aio-local.
@JeanMeche JeanMeche restored the fix/in-memory-with-fetch branch November 2, 2023 19:25
@JeanMeche JeanMeche reopened this Nov 2, 2023
@JeanMeche JeanMeche force-pushed the fix/in-memory-with-fetch branch from 1b8334d to 6de5db5 Compare November 2, 2023 19:26
@JeanMeche JeanMeche added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Nov 2, 2023
@JeanMeche
Copy link
Copy Markdown
Member Author

JeanMeche commented Nov 2, 2023

The initial merge was reverted in #52425 because of test regressions.

Those are now fixed.

@JeanMeche JeanMeche force-pushed the fix/in-memory-with-fetch branch from 6de5db5 to cacae2e Compare November 2, 2023 23:06
@JeanMeche
fix(http): Don't override the backend when using the InMemoryWebAPI
6e85cc6 
When using `withFetch`,  the `PRIMARY_HTTP_BACKEND` token is set.

The InMemory Backend services will also set that token.

This means that providers order will matter and the latest on the list will be the one instantiated
@JeanMeche JeanMeche force-pushed the fix/in-memory-with-fetch branch from cacae2e to 6e85cc6 Compare November 2, 2023 23:31
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 6, 2023
alxhub added a commit to alxhub/angular that referenced this pull request Nov 8, 2023
@alxhub
Revert "fix(http): Don't override the backend when using the InMemory…
169f4b7 
…WebAPI (angular#52425)"

This reverts commit 49b037f. Reason: it
breaks tests in aio-local.
pkozlowski-opensource pushed a commit that referenced this pull request Nov 8, 2023
@alxhub @pkozlowski-opensource
Revert "fix(http): Don't override the backend when using the InMemory…
da429c5 
…WebAPI (#52425)" (#52630)

This reverts commit 49b037f. Reason: it
breaks tests in aio-local.

PR Close #52630
@dylhunn dylhunn added target: patch This PR is targeted for the next patch release and removed target: rc This PR is targeted for the next release-candidate labels Nov 8, 2023
@jessicajaniuk
Copy link
Copy Markdown
Contributor

jessicajaniuk commented Nov 8, 2023

This PR was merged into the repository by commit 291ba38.

jessicajaniuk pushed a commit that referenced this pull request Nov 8, 2023
@JeanMeche @thePunderWoman
fix(http): Don't override the backend when using the InMemoryWebAPI (#…
5c6f3f8 
…52425)

When using `withFetch`,  the `PRIMARY_HTTP_BACKEND` token is set.

The InMemory Backend services will also set that token.

This means that providers order will matter and the latest on the list will be the one instantiated

PR Close #52425
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Dec 9, 2023

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 Dec 9, 2023
@JeanMeche JeanMeche deleted the fix/in-memory-with-fetch branch December 24, 2023 14:39
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
fix(http): Don't override the backend when using the InMemoryWebAPI (a…
6104988 
…ngular#52425)

When using `withFetch`,  the `PRIMARY_HTTP_BACKEND` token is set.

The InMemory Backend services will also set that token.

This means that providers order will matter and the latest on the list will be the one instantiated

PR Close angular#52425
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
fix(http): Don't override the backend when using the InMemoryWebAPI (a…
d6e1063 
…ngular#52425)

When using `withFetch`,  the `PRIMARY_HTTP_BACKEND` token is set.

The InMemory Backend services will also set that token.

This means that providers order will matter and the latest on the list will be the one instantiated

PR Close angular#52425
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alan-agius4 alan-agius4 alan-agius4 left review comments

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

+1 more reviewer

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: common/http Issues related to HTTP and HTTP Client target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

6 participants

@JeanMeche @dylhunn @alxhub @jessicajaniuk @alan-agius4 @AndrewKushnir