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

fix(http): use serializeBody to support JSON payload in FetchBackend #50776

Closed
wants to merge 1 commit into from
Closed

fix(http): use serializeBody to support JSON payload in FetchBackend #50776

wants to merge 1 commit into from

Conversation

leo6104
Copy link
Contributor

@leo6104 leo6104 commented Jun 20, 2023 •
edited

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #50775

What is the new behavior?

it correctly serialize request body (same behavior with HttpXhrBackend)

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from AndrewKushnir June 20, 2023 05:59
@leo6104 leo6104 force-pushed the fix/fetch-body branch 3 times, most recently from 7ee46f0 to 7925440 Compare June 20, 2023 06:31
@leo6104 leo6104 changed the title fix(fetch): use serializeBody() to support JSON payload fix(http): use serializeBody to support JSON payload in FetchBackend Jun 20, 2023
@pkozlowski-opensource pkozlowski-opensource added action: review The PR is still awaiting reviews from at least one requested reviewer area: common/http labels Jun 20, 2023
@ngbot ngbot bot added this to the Backlog milestone Jun 20, 2023
@pkozlowski-opensource
Copy link
Member

I would expect to see a test verifying code in this PR.

@leo6104 leo6104 closed this Jun 20, 2023
@leo6104 leo6104 reopened this Jun 20, 2023
@leo6104
fix(http): use serializeBody to support JSON payload in FetchBackend
4d4f8c4 
`HttpRequest.serializeBody` was used in HttpXhrBackend. `fetch` also needs to serialize request body.

Close angular#50775
@leo6104
Copy link
Contributor Author

leo6104 commented Jun 20, 2023

@pkozlowski-opensource Thanks for comment. 😊

I adds test codes in XHR / Fetch both side.

@AndrewKushnir AndrewKushnir added the target: patch This PR is targeted for the next patch release label Jun 20, 2023
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 20, 2023
AndrewKushnir
AndrewKushnir approved these changes Jun 20, 2023
View reviewed changes
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.

@leo6104 thanks for the fix 👍

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Jun 20, 2023
@ngbot
Copy link

ngbot bot commented Jun 20, 2023

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google-internal-tests" is failing
    pending status "mergeability" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit a126cbc.

AndrewKushnir pushed a commit that referenced this pull request Jun 20, 2023
@leo6104 @AndrewKushnir
fix(http): use serializeBody to support JSON payload in FetchBackend (#…
5ae0018 
…50776)

`HttpRequest.serializeBody` was used in HttpXhrBackend. `fetch` also needs to serialize request body.

Close #50775

PR Close #50776
@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 Jul 21, 2023
thomasturrell pushed a commit to thomasturrell/angular that referenced this pull request Aug 29, 2023
@leo6104
fix(http): use serializeBody to support JSON payload in FetchBackend (a…
908f733 
…ngular#50776)

`HttpRequest.serializeBody` was used in HttpXhrBackend. `fetch` also needs to serialize request body.

Close angular#50775

PR Close angular#50776
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@leo6104 @ChellappanRajan
fix(http): use serializeBody to support JSON payload in FetchBackend (a…
236ff9a 
…ngular#50776)

`HttpRequest.serializeBody` was used in HttpXhrBackend. `fetch` also needs to serialize request body.

Close angular#50775

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

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: common/http 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.

None yet
3 participants
@leo6104 @pkozlowski-opensource @AndrewKushnir