Merge pull request #1067 from Shopify/refactor_adapter_fetch

Refactor runtime adapter abstract fetch

.changeset/rich-ladybugs-own.md

@@ -0,0 +1,11 @@
+---
+"@shopify/shopify-api": major
+---
+
+> [!NOTE]
+> This change only affects apps that are using custom runtime adapters.
+> If you're using a default adapter from this package, you don't need to make this change.
+
+Changed `setAbstractFetchFunc` to accept a `fetch` API instead of one based on `NormalizedRequest` and `NormalizedResponse`.
+
+With this change, we can return a `Response` object for requests with the upcoming clients, which can help make the interface for requests more familiar to users.

packages/shopify-api/adapters/cf-worker/index.ts

@@ -9,12 +9,11 @@ import {
   webApiConvertHeaders,
   webApiConvertRequest,
   webApiConvertResponse,
-  webApiFetch,
 } from '../web-api/adapter';
 
 import {workerRuntimeString} from './adapter';
 
-setAbstractFetchFunc(webApiFetch);
+setAbstractFetchFunc(fetch);
 setAbstractConvertRequestFunc(webApiConvertRequest);
 setAbstractConvertResponseFunc(webApiConvertResponse);
 setAbstractConvertHeadersFunc(webApiConvertHeaders);

packages/shopify-api/adapters/mock/adapter.ts

@@ -1,4 +1,12 @@
 import {
+  Headers as FetchHeaders,
+  Request,
+  RequestInit,
+  Response,
+} from 'node-fetch';
+
+import {
+  AbstractFetchFunc,
   AdapterArgs,
   AdapterHeaders,
   canonicalizeHeaders,
@@ -33,30 +41,45 @@ export async function mockConvertHeaders(
   return Promise.resolve(headers);
 }
 
-export async function mockFetch({
-  url,
-  method,
-  headers = {},
-  body,
-}: NormalizedRequest): Promise<NormalizedResponse> {
+export const mockFetch: AbstractFetchFunc = async (url, init) => {
+  const mockInit = init as RequestInit;
+
+  const request = new Request(url as string, mockInit);
+  const headers = Object.fromEntries(
+    new FetchHeaders(mockInit?.headers).entries(),
+  );
+
   mockTestRequests.requestList.push({
-    url,
-    method,
+    url: request.url,
+    method: request.method,
     headers: canonicalizeHeaders(headers),
-    body,
+    body: await request.text(),
   });
 
   const next = mockTestRequests.responseList.shift()!;
   if (!next) {
     throw new Error(
-      `Missing mock for ${method} to ${url}, have you queued all required responses?`,
+      `Missing mock for ${request.method} to ${url}, have you queued all required responses?`,
     );
   }
   if (next instanceof Error) {
     throw next;
   }
-  return next;
-}
+
+  const responseHeaders = new FetchHeaders();
+  Object.entries(next.headers ?? {}).forEach(([key, value]) => {
+    responseHeaders.set(
+      key,
+      typeof value === 'string' ? value : value.join(', '),
+    );
+  });
+
+  return new Response(next.body, {
+    status: next.statusCode,
+    statusText: next.statusText,
+    headers: responseHeaders as any,
+  }) as any;
+};
 
 export function mockRuntimeString() {
   return 'Mock adapter';

packages/shopify-api/adapters/node/adapter.ts

@@ -1,11 +1,8 @@
 import type {IncomingMessage, ServerResponse} from 'http';
 
-import fetch from 'node-fetch';
-
 import {
   AdapterArgs,
   canonicalizeHeaders,
-  flatHeaders,
   Headers,
   NormalizedRequest,
   NormalizedResponse,
@@ -72,22 +69,6 @@ export async function nodeConvertAndSetHeaders(
   );
 }
 
-export async function nodeFetch({
-  url,
-  method,
-  headers = {},
-  body,
-}: NormalizedRequest): Promise<NormalizedResponse> {
-  const resp = await fetch(url, {method, headers: flatHeaders(headers), body});
-  const respBody = await resp.text();
-  return {
-    statusCode: resp.status,
-    statusText: resp.statusText,
-    body: respBody,
-    headers: canonicalizeHeaders(Object.fromEntries(resp.headers.entries())),
-  };
-}
-
 export function nodeRuntimeString() {
   return `Node ${process.version}`;
 }

packages/shopify-api/adapters/node/index.ts

@@ -1,5 +1,7 @@
 import crypto from 'crypto';
 
+import fetch from 'node-fetch';
+
 import {
   setAbstractFetchFunc,
   setAbstractConvertRequestFunc,
@@ -8,18 +10,19 @@ import {
   setAbstractConvertHeadersFunc,
   setAbstractRuntimeString,
   setCrypto,
+  AbstractFetchFunc,
 } from '../../runtime';
 
 import {
-  nodeFetch,
   nodeConvertRequest,
   nodeConvertIncomingResponse,
   nodeConvertAndSendResponse,
   nodeConvertAndSetHeaders,
   nodeRuntimeString,
 } from './adapter';
 
-setAbstractFetchFunc(nodeFetch);
+// For the purposes of this package, fetch correctly implements everything we need
+setAbstractFetchFunc(fetch as any as AbstractFetchFunc);
 setAbstractConvertRequestFunc(nodeConvertRequest);
 setAbstractConvertIncomingResponseFunc(nodeConvertIncomingResponse);
 setAbstractConvertResponseFunc(nodeConvertAndSendResponse);

packages/shopify-api/adapters/web-api/adapter.ts

@@ -4,7 +4,7 @@ import type {
   NormalizedResponse,
   NormalizedRequest,
 } from '../../runtime';
-import {addHeader, canonicalizeHeaders, flatHeaders} from '../../runtime';
+import {addHeader, flatHeaders} from '../../runtime';
 
 interface WebApiAdapterArgs extends AdapterArgs {
   rawRequest: Request;
@@ -48,26 +48,6 @@ export async function webApiConvertResponse(
   });
 }
 
-export async function webApiFetch({
-  headers,
-  method,
-  url,
-  body,
-}: NormalizedRequest): Promise<NormalizedResponse> {
-  const resp = await fetch(url, {
-    method,
-    headers: flatHeaders(headers),
-    body,
-  });
-  const respBody = await resp.text();
-  return {
-    statusCode: resp.status,
-    statusText: resp.statusText,
-    body: respBody,
-    headers: canonicalizeHeaders(Object.fromEntries(resp.headers.entries())),
-  };
-}
-
 export function webApiRuntimeString(): string {
   return 'Web API';
 }

packages/shopify-api/adapters/web-api/index.ts

@@ -10,11 +10,10 @@ import {
   webApiConvertHeaders,
   webApiConvertRequest,
   webApiConvertResponse,
-  webApiFetch,
   webApiRuntimeString,
 } from './adapter';
 
-setAbstractFetchFunc(webApiFetch);
+setAbstractFetchFunc(fetch);
 setAbstractConvertRequestFunc(webApiConvertRequest);
 setAbstractConvertResponseFunc(webApiConvertResponse);
 setAbstractConvertHeadersFunc(webApiConvertHeaders);

packages/shopify-api/docs/migrating-to-v9.md

@@ -0,0 +1,71 @@
+# Migrating to v9
+
+This document covers the changes apps will need to make to be able to upgrade to v9 of this package.
+
+In this major version, our focus has been to integrate the `@shopify/shopify-api` package with our new GraphQL API clients, also contained in this repository.
+
+> [!NOTE]
+> This change is only breaking for apps that have created a custom runtime adapter that calls `setAbstractFetchFunc`.
+> All our existing adapters were already updated accordingly.
+
+To make it easier to navigate this guide, here is an overview of the sections it contains:
+
+- [Migrating to v9](#migrating-to-v9)
+  - [Changes to runtime adapters](#changes-to-runtime-adapters)
+
+---
+
+## Changes to runtime adapters
+
+To better integrate with the new clients' ability to return the [Web API fetch](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API) response, our adapters were slightly changed: the `setAbstractFetchFunc` will now match the `fetch` API.
+
+That means that instead of accepting a `NormalizedRequest` and returning a `NormalizedResponse`, it'll accept the `url` and `RequestInit` params, and return a `Response`.
+
+This change enables us to return the raw `Response` object which might be more familiar to developers, and it makes creating new runtime adapters easier because you can pass in a `fetch` implementation directly, if one is available, bringing us closer to the community standard.
+
+Before:
+
+```ts
+import {setAbstractFetchFunc} from '@shopify/shopify-api/runtime';
+
+export async function nodeFetch({
+  url,
+  method,
+  headers = {},
+  body,
+}: NormalizedRequest): Promise<NormalizedResponse> {
+  const resp = await fetch(url, {method, headers: flatHeaders(headers), body});
+  const respBody = await resp.text();
+  return {
+    statusCode: resp.status,
+    statusText: resp.statusText,
+    body: respBody,
+    headers: canonicalizeHeaders(Object.fromEntries(resp.headers.entries())),
+  };
+}
+
+setAbstractFetchFunc(nodeFetch);
+```
+
+After:
+
+```ts
+import fetch from 'node-fetch';
+import {setAbstractFetchFunc} from '@shopify/shopify-api/runtime';
+
+setAbstractFetchFunc(fetch);
+```
+
+or, if a `fetch` implementation isn't available:
+
+```ts
+import {AbstractFetchFunc, setAbstractFetchFunc} from '@shopify/shopify-api/runtime';
+
+const convertFetch: AbstractFetchFunc = (url, init) => {
+  // Make the actual request
+
+  return new Response(/* ... */);
+}
+
+setAbstractFetchFunc(convertFetch);
+```

packages/shopify-api/lib/clients/http_client/__tests__/http_client.test.ts

@@ -49,17 +49,17 @@ describe('HTTP client', () => {
     }).toMatchMadeHttpRequest();
   });
 
-  it('allows the body to contain non-json 2xx response without dying', () => {
+  it('allows the body to contain non-json 2xx response without dying', async () => {
     const client = new HttpClient({domain});
     queueMockResponse('not a json object');
 
     const request = client.get({path: '/url/path'});
 
+    await expect(request).resolves.toMatchObject({body: {}});
     expect({method: 'GET', domain, path: '/url/path'}).toMatchMadeHttpRequest();
-    expect(request).resolves.toMatchObject({body: {}});
   });
 
-  it('handles non-json non-2xx response', () => {
+  it('handles non-json non-2xx response', async () => {
     const client = new HttpClient({domain});
     queueMockResponse('not a json object', {
       statusCode: 404,
@@ -69,8 +69,10 @@ describe('HTTP client', () => {
 
     const request = client.get({path: '/url/path'});
 
+    await expect(request).rejects.toBeInstanceOf(
+      ShopifyErrors.HttpResponseError,
+    );
     expect({method: 'GET', domain, path: '/url/path'}).toMatchMadeHttpRequest();
-    expect(request).rejects.toBeInstanceOf(ShopifyErrors.HttpResponseError);
   });
 
   it('can make POST request with type JSON', async () => {

packages/shopify-api/lib/clients/http_client/http_client.ts

@@ -10,6 +10,7 @@ import {HashFormat} from '../../../runtime/crypto/types';
 import {
   abstractFetch,
   canonicalizeHeaders,
+  flatHeaders,
   getHeader,
   isOK,
   NormalizedRequest,
@@ -272,7 +273,7 @@ export class HttpClient {
   ): Promise<RequestReturn<T>> {
     const log = logger(this.httpClass().config);
 
-    const response: NormalizedResponse = await abstractFetch(request);
+    const response: NormalizedResponse = await normalizedFetch(request);
 
     if (this.httpClass().config.logger.httpRequests) {
       log.debug(
@@ -352,3 +353,23 @@ export function httpClientClass(
 
   return NewHttpClient as typeof HttpClient;
 }
+
+export async function normalizedFetch({
+  headers,
+  method,
+  url,
+  body,
+}: NormalizedRequest): Promise<NormalizedResponse> {
+  const resp = await abstractFetch(url, {
+    method,
+    headers: flatHeaders(headers),
+    body,
+  });
+  const respBody = await resp.text();
+  return {
+    statusCode: resp.status,
+    statusText: resp.statusText,
+    body: respBody,
+    headers: canonicalizeHeaders(Object.fromEntries(resp.headers.entries())),
+  };
+}

packages/shopify-api/lib/clients/rest/__tests__/resources/base.test.ts

@@ -113,11 +113,11 @@ describe('Base REST resource', () => {
       headers: {'X-Test-Header': 'value'},
     });
 
-    const expectedError = await expect(
+    const expectedError = expect(
       shopify.rest.FakeResource.find({id: 1, session}),
     ).rejects;
-    expectedError.toThrowError(HttpResponseError);
-    expectedError.toMatchObject({
+    await expectedError.toThrowError(HttpResponseError);
+    await expectedError.toMatchObject({
       response: {
         body: {errors: 'Not Found'},
         code: 404,

packages/shopify-api/runtime/http/types.ts

@@ -24,9 +24,7 @@ export interface AdapterArgs {
   rawResponse?: AdapterResponse;
 }
 
-export type AbstractFetchFunc = (
-  req: NormalizedRequest,
-) => Promise<NormalizedResponse>;
+export type AbstractFetchFunc = typeof fetch;
 
 export type AbstractConvertRequestFunc = (
   adapterArgs: AdapterArgs,