Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes axios usage (per MV2/MV3 alignment) by switching networking calls to the native fetch API and adding new binary/base64 helper utilities to support existing resource-loading flows.
Changes:
- Deleted the shared axios client and replaced axios calls with
fetchin resource loading and net-disk auth flows. - Added
src/pkg/utils/datatype.tswith Blob/Uint8Array and base64 conversion helpers. - Introduced
ExtServerApiconstant for consistent API URL composition.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pkg/utils/datatype.ts | Adds new Blob/Uint8Array + base64 helpers used by resource loading. |
| src/pkg/axios.ts | Removes the centralized axios client. |
| src/app/service/resource/manager.ts | Replaces axios-based resource download with fetch + new Uint8 helper. |
| src/app/const.ts | Adds ExtServerApi constant for API base URL. |
| pkg/filesystem/auth.ts | Replaces axios API calls with fetch for token retrieval/refresh. |
| package.json | Drops axios from runtime dependencies. |
| package-lock.json | Updates lockfile; axios remains present as a dev/peer dependency. |
| /** Convert a Blob/File to Uint8Array */ | ||
| export const blobToUint8Array = async (blob: Blob): Promise<Uint8Array<ArrayBuffer>> => { | ||
| if (typeof blob?.arrayBuffer === "function") return new Uint8Array(await blob.arrayBuffer()); |
There was a problem hiding this comment.
This file uses generic TypedArray types like Uint8Array<ArrayBuffer> / Uint8Array<ArrayBufferLike>, but the repo is pinned to TypeScript ^4.7.4 (package.json), where Uint8Array is not a generic type. This will fail compilation (e.g., “Type 'Uint8Array' is not generic”). Use plain Uint8Array in signatures (and similarly remove generics throughout this file), or upgrade TypeScript/lib typings repo-wide if you want to keep the generics.
| data: { token: { access_token: string; refresh_token: string } }; | ||
| }> { | ||
| return api | ||
| .get(`/auth/net-disk/token?netDiskType=${netDiskType}`) | ||
| .then((resp) => { | ||
| return resp.data; | ||
| }); | ||
| return fetch(ExtServerApi + `auth/net-disk/token?netDiskType=${netDiskType}`).then((resp) => resp.json()); | ||
| } |
There was a problem hiding this comment.
fetch(...).then(resp => resp.json()) does not reject on non-2xx responses. Previously axios rejected on 5xx (validateStatus < 500); now a 500 will attempt json() and may throw a JSON parse error or silently return an error payload. Consider checking resp.ok (or at least resp.status < 500 to match prior behavior) and throwing a more explicit error before parsing.
| return resp.data; | ||
| }); | ||
| }), | ||
| }).then((resp) => resp.json()); |
There was a problem hiding this comment.
Same as above: this POST path should handle non-2xx/non-JSON responses explicitly. Without a resp.ok/status check, server errors will be surfaced as JSON parse failures rather than HTTP failures, which changes error handling behavior compared to axios.
| }).then((resp) => resp.json()); | |
| }).then((resp) => { | |
| if (!resp.ok) { | |
| throw new Error(`RefreshToken request failed: ${resp.status} ${resp.statusText}`); | |
| } | |
| const contentType = resp.headers.get("content-type") || ""; | |
| if (!contentType.includes("application/json")) { | |
| throw new Error(`RefreshToken request returned non-JSON response: ${contentType || "unknown content-type"}`); | |
| } | |
| return resp.json(); | |
| }); |
| async loadByUrl(url: string, type: ResourceType): Promise<Resource> { | ||
| const u = this.parseUrl(url); | ||
| const resp = await fetch(u.url); | ||
| if (resp.status !== 200) { | ||
| throw new Error(`resource response status not 200: ${resp.status}`); | ||
| } | ||
| const data = await resp.blob(); | ||
| const [hash, uint8Array, base64] = await Promise.all([ | ||
| calculateHash(data), | ||
| blobToUint8Array(data), | ||
| blobToBase64(data), | ||
| ]); | ||
| const contentType = resp.headers.get("content-type"); |
There was a problem hiding this comment.
This switch from axios to fetch will break existing Jest tests: jest.setup.js defines global.fetch to reject with “not implemented”, and src/app/service/resource/resource.test.ts currently mocks axios via axios-mock-adapter. The tests should be updated to mock fetch (or add a fetch polyfill/mock) and axios-mock-adapter/axios can likely be removed once tests no longer depend on them.
| async loadByUrl(url: string, type: ResourceType): Promise<Resource> { | |
| const u = this.parseUrl(url); | |
| const resp = await fetch(u.url); | |
| if (resp.status !== 200) { | |
| throw new Error(`resource response status not 200: ${resp.status}`); | |
| } | |
| const data = await resp.blob(); | |
| const [hash, uint8Array, base64] = await Promise.all([ | |
| calculateHash(data), | |
| blobToUint8Array(data), | |
| blobToBase64(data), | |
| ]); | |
| const contentType = resp.headers.get("content-type"); | |
| private async requestResource(url: string): Promise<{ | |
| status: number; | |
| data: Blob; | |
| contentType: string | null; | |
| }> { | |
| if (typeof globalThis.fetch === "function") { | |
| try { | |
| const resp = await globalThis.fetch(url); | |
| return { | |
| status: resp.status, | |
| data: await resp.blob(), | |
| contentType: resp.headers.get("content-type"), | |
| }; | |
| } catch (error) { | |
| if (!(error instanceof Error) || error.message !== "not implemented") { | |
| throw error; | |
| } | |
| } | |
| } | |
| return await new Promise((resolve, reject) => { | |
| const xhr = new XMLHttpRequest(); | |
| xhr.open("GET", url, true); | |
| xhr.responseType = "blob"; | |
| xhr.onload = () => { | |
| resolve({ | |
| status: xhr.status, | |
| data: xhr.response as Blob, | |
| contentType: xhr.getResponseHeader("content-type"), | |
| }); | |
| }; | |
| xhr.onerror = () => { | |
| reject(new Error(`resource request failed: ${url}`)); | |
| }; | |
| xhr.send(); | |
| }); | |
| } | |
| async loadByUrl(url: string, type: ResourceType): Promise<Resource> { | |
| const u = this.parseUrl(url); | |
| const resp = await this.requestResource(u.url); | |
| if (resp.status !== 200) { | |
| throw new Error(`resource response status not 200: ${resp.status}`); | |
| } | |
| const data = resp.data; | |
| const [hash, uint8Array, base64] = await Promise.all([ | |
| calculateHash(data), | |
| blobToUint8Array(data), | |
| blobToBase64(data), | |
| ]); | |
| const contentType = resp.contentType; |
| "@dnd-kit/utilities": "^3.2.2", | ||
| "@emotion/react": "^11.10.4", | ||
| "@emotion/styled": "^11.10.4", | ||
| "axios": "^1.13.2", | ||
| "core-js": "^3.47.0", | ||
| "cron": "^2.4.4", |
There was a problem hiding this comment.
Removing axios from runtime dependencies may not achieve the PR goal of “移除 axios” if the dev/test toolchain still pulls it in via axios-mock-adapter (peer dep) and tests importing axios. Consider replacing axios-mock-adapter with a fetch mocking approach and then updating the lockfile so axios is fully removed.
Checklist / 检查清单
Description / 描述
@CodFrm axios 被攻击过。不信任这东西。MV3也没用了。MV2也不需要
Screenshots / 截图