Use fetch helpers instead of fetch #27026

silverwind · 2023-09-11T17:14:00Z

WIP because:

Some calls set a content-type but send no body, can likely remove the header
Need to check whether charset=utf-8 has any significance on the webauthn calls, I assume not as it is the default for json content.
Maybe no-restricted-globals is better for eslint, but will require a lot of duplication in the yaml or moving eslint config to a .js extension.
Maybe export request as fetch, shadowing the global.

wxiaoguang · 2023-09-12T00:38:30Z

Maybe export request as fetch, shadowing the global.

I am against doing so, it would pollute the names and reduce maintainbility. Think about a real case: a developer copy&paste a block of fetch code into Gitea's JS file, but Gitea's JS imports the self-made fetch, then it would surprise the developer why the fetch behavior is different.

And such a general and common word makes it different to search&replace&refactor.

web_src/js/modules/fetch.js

silverwind · 2023-09-12T15:31:11Z

I am against doing so, it would pollute the names and reduce maintainbility. Think about a real case: a developer copy&paste a block of fetch code into Gitea's JS file, but Gitea's JS imports the self-made fetch, then it would surprise the developer why the fetch behavior is different.

Generally our request is designed to work like fetch, but I tend to agree, shadowing is bad in case of globals, even thought there is an escape hatch via window.fetch.

web_src/js/features/repo-migrate.js

web_src/js/modules/fetch.js

* origin/main: (53 commits) Search branches (go-gitea#27055) Fix wrong migration for email address (go-gitea#27106) [skip ci] Updated translations via Crowdin Support `.git-blame-ignore-revs` file (go-gitea#26395) Add `RemoteAddress` to mirrors (go-gitea#26952) Upgrading the actions/checkout@4 (go-gitea#27096) Next round of `db.DefaultContext` refactor (go-gitea#27089) Ui correction in mobile view nav bar left aligned items. (go-gitea#27046) Add missing deps to files-changed (go-gitea#27100) Use db.WithTx for AddTeamMember to avoid ctx abuse (go-gitea#27095) Drop Node.js 16 and update js dependencies (go-gitea#27094) Fix NPE when editing OAuth2 applications (go-gitea#27078) Use `print` instead of `printf` (go-gitea#27093) Add tests for db indexer in indexer_test.go (go-gitea#27087) [skip ci] Updated translations via Crowdin Allow empty Conan files (go-gitea#27092) Actions are no longer experimental, so enable them by default (go-gitea#27054) Update brew installation documentation since gitea moved to brew core package (go-gitea#27070) More refactoring of `db.DefaultContext` (go-gitea#27083) [skip ci] Updated translations via Crowdin ...

silverwind · 2023-09-17T21:43:16Z

There is at least one bug with POST /user/repo/pulls/1/pin where the form-fetch-actions currently sends application/json content-type while actually sending a multipart/form-data body. This PR makes it send the correct header, but it results in a JSON.parse error currently.

silverwind · 2023-09-17T21:58:52Z

Actually no, the issue was different. Because form-fetch-action set Header X-Csrf-Token and request set the same lowercase, fetch decided to merge the headers into one value <token1>, <token2> which the backend could not parse. I'll see to merging these headers case-insensitively to avoid such issues.

silverwind · 2023-09-17T22:05:43Z

This showcases the issue. Imho, a flaw in the fetch spec that these merge into the value, not replace:

silverwind · 2023-09-17T22:22:09Z

Another issue fixed related to FormData: The boundary parameter in content-type is required, so we must actually not set the content-type header manually, fetch will do it automatically when the body is FormData: https://stackoverflow.com/a/39281156/808699. Checking if the same works for URLSearchParams.

silverwind · 2023-09-17T22:25:43Z

Confirmed: When leaving out the content-type header for URLSearchParams, the browser will automatically set application/x-www-form-urlencoded;charset=UTF-8, e.g. with charset so that @delvh is happy too.

web_src/js/modules/fetch.js

web_src/js/features/pull-view-file.js

silverwind · 2023-09-17T22:41:43Z

This is ready. I manually confirmed webauthn and repo migration actions still work.

web_src/js/modules/fetch.js

* giteaofficial/main: Improve actions docs related to `pull_request` event (go-gitea#27126) Remove outdated paragraphs when comparing Gitea Actions to GitHub Actions (go-gitea#27119) Fix: treat tab "overview" as "repositories" in user profiles without readme (go-gitea#27124) Fix incorrect test code for error handling (go-gitea#27139) Increase auth provider icon size on login page (go-gitea#27122) fix pagination for followers and following (go-gitea#27127) services/wiki: Close() after error handling (go-gitea#27129) Use fetch helpers instead of fetch (go-gitea#27026) Change green buttons to primary color (go-gitea#27099) Fix wrong xorm get usage on migration (go-gitea#27111) Fix the incorrect route path in the user edit page. (go-gitea#27007) Refactor lfs requests (go-gitea#26783) Display archived labels specially when listing labels (go-gitea#26820) Remove a `gt-float-right` and some unnecessary helpers (go-gitea#27110) [skip ci] Updated licenses and gitignores Fix token endpoints ignore specified account (go-gitea#27080) Make SSPI auth mockable (go-gitea#27036)

* origin/main: Fix dropdown icon position (go-gitea#27175) Fix repo sub menu (go-gitea#27169) Fix review request number and add more tests (go-gitea#27104) Fix the variable regexp pattern on web page (go-gitea#27161) Fix organization field being null in POST /orgs/{orgid}/teams (go-gitea#27150) Add index to `issue_user.issue_id` (go-gitea#27154) [skip ci] Updated translations via Crowdin Start development on Gitea 1.22 (go-gitea#27155) Fix successful return value for `SyncAndGetUserSpecificDiff` (go-gitea#27152) Improve actions docs related to `pull_request` event (go-gitea#27126) Remove outdated paragraphs when comparing Gitea Actions to GitHub Actions (go-gitea#27119) Fix: treat tab "overview" as "repositories" in user profiles without readme (go-gitea#27124) Fix incorrect test code for error handling (go-gitea#27139) Increase auth provider icon size on login page (go-gitea#27122) fix pagination for followers and following (go-gitea#27127) services/wiki: Close() after error handling (go-gitea#27129) Use fetch helpers instead of fetch (go-gitea#27026) Change green buttons to primary color (go-gitea#27099) Fix wrong xorm get usage on migration (go-gitea#27111)

Use fetch helpers instead of fetch

bb83d4e

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 11, 2023

silverwind marked this pull request as draft September 11, 2023 17:14

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 11, 2023

github-actions bot added topic/ui Change the appearance of the Gitea UI topic/code-linting labels Sep 11, 2023

use set instead of append

1c1407d

silverwind added type/refactoring Existing code has been cleaned up. There should be no new functionality. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Sep 11, 2023

wxiaoguang reviewed Sep 12, 2023

View reviewed changes

web_src/js/modules/fetch.js Outdated Show resolved Hide resolved

silverwind mentioned this pull request Sep 13, 2023

Add fetch wrappers, ignore network errors in actions view #26985

Merged

delvh reviewed Sep 13, 2023

View reviewed changes

web_src/js/features/repo-migrate.js Outdated Show resolved Hide resolved

web_src/js/features/repo-migrate.js Outdated Show resolved Hide resolved

web_src/js/features/repo-migrate.js Outdated Show resolved Hide resolved

web_src/js/modules/fetch.js Show resolved Hide resolved

silverwind added 2 commits September 17, 2023 23:21

fix CSRF typo

c39a9b8

github-actions bot added the type/docs This PR mainly updates/creates documentation label Sep 17, 2023

silverwind added 3 commits September 17, 2023 23:23

remove _csrf set

fa14d8b

remove useless content-type

d4d82aa

remove DELETE body

c200586

remove unneccesary X-Csrf-Token

1e7220b

silverwind added 2 commits September 18, 2023 00:08

merge headers with .set

04370cf

fix form-data content-type

a898c83

don't set content-type on URLSearchParams

078d8b0

delvh reviewed Sep 17, 2023

View reviewed changes

web_src/js/modules/fetch.js Outdated Show resolved Hide resolved

silverwind commented Sep 17, 2023

View reviewed changes

web_src/js/features/pull-view-file.js Show resolved Hide resolved

silverwind marked this pull request as ready for review September 17, 2023 22:41

update comment

926357a

delvh approved these changes Sep 17, 2023

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 17, 2023

wxiaoguang reviewed Sep 18, 2023

View reviewed changes

web_src/js/modules/fetch.js Outdated Show resolved Hide resolved

silverwind added 2 commits September 19, 2023 00:51

define safe http methods and don't add csrf to them

7bcc203

remove query, not widely supported

708f095

wxiaoguang approved these changes Sep 19, 2023

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 19, 2023

silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 19, 2023

silverwind enabled auto-merge (squash) September 19, 2023 00:41

Merge branch 'main' into usefetch

938226c

silverwind merged commit ae8e8f0 into go-gitea:main Sep 19, 2023
26 checks passed

GiteaBot added this to the 1.22.0 milestone Sep 19, 2023

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 19, 2023

delvh modified the milestones: 1.22.0, 1.21.0 Sep 20, 2023

go-gitea locked as resolved and limited conversation to collaborators Dec 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use fetch helpers instead of fetch #27026

Use fetch helpers instead of fetch #27026

silverwind commented Sep 11, 2023 •

edited

wxiaoguang commented Sep 12, 2023

silverwind commented Sep 12, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited by delvh

silverwind commented Sep 17, 2023

Use fetch helpers instead of fetch #27026

Use fetch helpers instead of fetch #27026

Conversation

silverwind commented Sep 11, 2023 • edited

wxiaoguang commented Sep 12, 2023

silverwind commented Sep 12, 2023 • edited

silverwind commented Sep 17, 2023 • edited

silverwind commented Sep 17, 2023 • edited

silverwind commented Sep 17, 2023 • edited

silverwind commented Sep 17, 2023 • edited

silverwind commented Sep 17, 2023 • edited by delvh

silverwind commented Sep 17, 2023

silverwind commented Sep 11, 2023 •

edited

silverwind commented Sep 12, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited

silverwind commented Sep 17, 2023 •

edited by delvh