Skip to content

Downgrade api-fetch package to 7.29 #3662

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

Merged
acicovic merged 1 commit into from
Sep 8, 2025
Merged

Downgrade api-fetch package to 7.29 #3662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 166 additions & 6 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"@types/wordpress__edit-post": "^8.4.2",
"@types/wordpress__wordcount": "^2.4.5",
"@typescript-eslint/eslint-plugin": "^6.21.0",
"@wordpress/api-fetch": "^7.30.0",
"@wordpress/api-fetch": "7.29",
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 8, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Pin looks good; consider forcing a single version across the tree and documenting why.

  • To avoid a second copy being hoisted by transitive deps, consider pinning via overrides/resolutions as well and add a rationale comment (similar to others) to prevent accidental upgrades back to 7.30+.

Apply:

   "devDependencies": {
-    "@wordpress/api-fetch": "7.29",
+    "@wordpress/api-fetch": "7.29",
   },
+  "devDependenciesComments": {
+    "@wordpress/api-fetch": "Pinned to 7.29 to avoid stricter typing in 7.30 that breaks our build. Do not upgrade until types are refactored."
+  },
   "overrides": {
-    "puppeteer-core": "^23.1.0"
+    "puppeteer-core": "^23.1.0",
+    "@wordpress/api-fetch": "7.29"
   },

Verify no other copy of api-fetch is brought in and no lingering APIFetchOptions remain:

🏁 Script executed:

#!/bin/bash
# Look for any usage of the removed generic.
rg -nP --type=ts --type=tsx 'APIFetchOptions\s*<\s*true\s*>' -C2

# If a lockfile exists, ensure only 7.29 is resolved.
fd -tf 'package-lock.json|pnpm-lock.yaml|yarn.lock' | xargs -I{} rg -n 'api-fetch@|@wordpress/api-fetch' -n {}

Length of output: 2630

Force single @wordpress/api-fetch version and document rationale
Multiple transitive deps still pull in @wordpress/api-fetch@7.30.x and @wordpress/api-fetch@6.55.x; add a Yarn resolutions block to pin all instances to 7.29.0 and include a comment:

 "resolutions": {
-  // existing overrides...
+  "@wordpress/api-fetch": "7.29.0"
 },
+ "devDependenciesComments": {
+  "@wordpress/api-fetch": "Pinned to 7.29.0 to avoid stricter typing in 7.30+ that breaks our build"
+ }

No occurrences of APIFetchOptions<true> remain.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In package.json around line 57, force all transitive instances of
@wordpress/api-fetch to 7.29.0 by adding a Yarn "resolutions" block that pins
"@wordpress/api-fetch" to "7.29.0" and include a one-line comment above it
explaining the rationale ("pin @wordpress/api-fetch to 7.29.0 to avoid mixed
incompatible versions across transitive deps"); after adding the resolution, run
yarn install to update the lockfile and verify no occurrences of
APIFetchOptions<true> remain.

"@wordpress/babel-preset-default": "^7.42.0",
"@wordpress/block-editor": "^15.3.0",
"@wordpress/blocks": "^15.3.0",
Expand Down
7 changes: 2 additions & 5 deletions src/content-helper/common/providers/base-provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,21 +137,18 @@ export abstract class BaseProvider {
* AbortController signal.
*
* @since 3.15.0
* @since 3.20.7 Using APIFetchOptions<true> to avoid a type error when building on GitHub.
*
* @param {APIFetchOptions} options The options to pass to apiFetch.
* @param {string?} id The (optional) ID of the request.
*
* @return {Promise<ContentHelperAPIResponse<any>>} The fetched data.
*/
protected async fetch<T>( options: APIFetchOptions<true>, id?: string ): Promise<T> {
protected async fetch<T>( options: APIFetchOptions, id?: string ): Promise<T> {
const { abortController, abortId } = this.getOrCreateController( id );
options.signal = abortController.signal;

try {
const response = await apiFetch<ContentHelperAPIResponse<T>>(
options as APIFetchOptions<true>
);
const response = await apiFetch<ContentHelperAPIResponse<T>>( options );

Comment on lines +146 to 152
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 8, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Type relaxation reintroduces a runtime hazard; enforce parse=true and avoid mutating caller options.

By dropping APIFetchOptions, callers can now pass parse:false and this method would still treat the result as parsed JSON, causing runtime errors. Also, mutating options in place can surprise callers.

Apply:

-	protected async fetch<T>( options: APIFetchOptions, id?: string ): Promise<T> {
+	protected async fetch<T>( options: APIFetchOptions, id?: string ): Promise<T> {
 		const { abortController, abortId } = this.getOrCreateController( id );
-
-		options.signal = abortController.signal;
+		// Enforce JSON parsing and avoid mutating the caller-provided object.
+		const fetchOptions: APIFetchOptions = {
+			...options,
+			signal: abortController.signal,
+			parse: true,
+		};
 
 		try {
-			const response = await apiFetch<ContentHelperAPIResponse<T>>( options );
+			const response = await apiFetch<ContentHelperAPIResponse<T>>( fetchOptions );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected async fetch<T>( options: APIFetchOptions, id?: string ): Promise<T> {
const { abortController, abortId } = this.getOrCreateController( id );
options.signal = abortController.signal;
try {
const response = await apiFetch<ContentHelperAPIResponse<T>>(
options as APIFetchOptions<true>
);
const response = await apiFetch<ContentHelperAPIResponse<T>>( options );
protected async fetch<T>( options: APIFetchOptions, id?: string ): Promise<T> {
const { abortController, abortId } = this.getOrCreateController( id );
// Enforce JSON parsing and avoid mutating the caller-provided object.
const fetchOptions: APIFetchOptions = {
...options,
signal: abortController.signal,
parse: true,
};
try {
const response = await apiFetch<ContentHelperAPIResponse<T>>( fetchOptions );
🤖 Prompt for AI Agents 
In src/content-helper/common/providers/base-provider.tsx around lines 146 to
152, the method relaxes APIFetchOptions allowing callers to pass parse:false and
then mutates their options, which can cause runtime parse errors; change the
method signature to accept APIFetchOptions<true> (enforcing parse=true), stop
mutating the caller's object by creating a shallow copy of options and set
parse:true on the copy (and preserve other fields), and use that copy when
calling apiFetch so the caller's original options remain unchanged.

// Validate API side errors.
if ( response.error ) {
Expand Down
Loading