From 64f61414cd1ace90e4cb96373be7739edb1c34f9 Mon Sep 17 00:00:00 2001 From: Paulo Margarido <64600052+paulomarg@users.noreply.github.com> Date: Tue, 28 Nov 2023 15:00:12 -0500 Subject: [PATCH 1/2] Refactor runtime adapter abstract fetch --- .changeset/rich-ladybugs-own.md | 13 ++++ .../shopify-api/adapters/cf-worker/index.ts | 3 +- packages/shopify-api/adapters/mock/adapter.ts | 47 ++++++++---- packages/shopify-api/adapters/node/adapter.ts | 19 ----- packages/shopify-api/adapters/node/index.ts | 7 +- .../shopify-api/adapters/web-api/adapter.ts | 22 +----- .../shopify-api/adapters/web-api/index.ts | 3 +- packages/shopify-api/docs/migrating-to-v9.md | 71 +++++++++++++++++++ .../http_client/__tests__/http_client.test.ts | 10 +-- .../lib/clients/http_client/http_client.ts | 23 +++++- .../rest/__tests__/resources/base.test.ts | 6 +- packages/shopify-api/runtime/http/types.ts | 4 +- 12 files changed, 159 insertions(+), 69 deletions(-) create mode 100644 .changeset/rich-ladybugs-own.md create mode 100644 packages/shopify-api/docs/migrating-to-v9.md diff --git a/.changeset/rich-ladybugs-own.md b/.changeset/rich-ladybugs-own.md new file mode 100644 index 000000000..6ed4cb696 --- /dev/null +++ b/.changeset/rich-ladybugs-own.md @@ -0,0 +1,13 @@ +--- +"@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. + +For more information and examples, see the [migration guide to v9](/packages/shopify-api/docs/migrating-to-v9.md#changes-to-runtime-adapters). diff --git a/packages/shopify-api/adapters/cf-worker/index.ts b/packages/shopify-api/adapters/cf-worker/index.ts index 2bc8f2c2d..d9c50150b 100644 --- a/packages/shopify-api/adapters/cf-worker/index.ts +++ b/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); diff --git a/packages/shopify-api/adapters/mock/adapter.ts b/packages/shopify-api/adapters/mock/adapter.ts index 96818a650..0575b88e3 100644 --- a/packages/shopify-api/adapters/mock/adapter.ts +++ b/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 { +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'; diff --git a/packages/shopify-api/adapters/node/adapter.ts b/packages/shopify-api/adapters/node/adapter.ts index 7942e46d0..48fac5a5d 100644 --- a/packages/shopify-api/adapters/node/adapter.ts +++ b/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 { - 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}`; } diff --git a/packages/shopify-api/adapters/node/index.ts b/packages/shopify-api/adapters/node/index.ts index 4652de15d..27b70552b 100644 --- a/packages/shopify-api/adapters/node/index.ts +++ b/packages/shopify-api/adapters/node/index.ts @@ -1,5 +1,7 @@ import crypto from 'crypto'; +import fetch from 'node-fetch'; + import { setAbstractFetchFunc, setAbstractConvertRequestFunc, @@ -8,10 +10,10 @@ import { setAbstractConvertHeadersFunc, setAbstractRuntimeString, setCrypto, + AbstractFetchFunc, } from '../../runtime'; import { - nodeFetch, nodeConvertRequest, nodeConvertIncomingResponse, nodeConvertAndSendResponse, @@ -19,7 +21,8 @@ import { 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); diff --git a/packages/shopify-api/adapters/web-api/adapter.ts b/packages/shopify-api/adapters/web-api/adapter.ts index 720b8b486..4463718a3 100644 --- a/packages/shopify-api/adapters/web-api/adapter.ts +++ b/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 { - 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'; } diff --git a/packages/shopify-api/adapters/web-api/index.ts b/packages/shopify-api/adapters/web-api/index.ts index 6101c6c10..267924558 100644 --- a/packages/shopify-api/adapters/web-api/index.ts +++ b/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); diff --git a/packages/shopify-api/docs/migrating-to-v9.md b/packages/shopify-api/docs/migrating-to-v9.md new file mode 100644 index 000000000..07dd565bb --- /dev/null +++ b/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 { + 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); +``` diff --git a/packages/shopify-api/lib/clients/http_client/__tests__/http_client.test.ts b/packages/shopify-api/lib/clients/http_client/__tests__/http_client.test.ts index 45cfd8194..ec13cd863 100644 --- a/packages/shopify-api/lib/clients/http_client/__tests__/http_client.test.ts +++ b/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 () => { diff --git a/packages/shopify-api/lib/clients/http_client/http_client.ts b/packages/shopify-api/lib/clients/http_client/http_client.ts index cce5783ad..42743010e 100644 --- a/packages/shopify-api/lib/clients/http_client/http_client.ts +++ b/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> { 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 { + 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())), + }; +} diff --git a/packages/shopify-api/lib/clients/rest/__tests__/resources/base.test.ts b/packages/shopify-api/lib/clients/rest/__tests__/resources/base.test.ts index bf0b344d0..d156762ca 100644 --- a/packages/shopify-api/lib/clients/rest/__tests__/resources/base.test.ts +++ b/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, diff --git a/packages/shopify-api/runtime/http/types.ts b/packages/shopify-api/runtime/http/types.ts index 8807b3e07..561103455 100644 --- a/packages/shopify-api/runtime/http/types.ts +++ b/packages/shopify-api/runtime/http/types.ts @@ -24,9 +24,7 @@ export interface AdapterArgs { rawResponse?: AdapterResponse; } -export type AbstractFetchFunc = ( - req: NormalizedRequest, -) => Promise; +export type AbstractFetchFunc = typeof fetch; export type AbstractConvertRequestFunc = ( adapterArgs: AdapterArgs, From 67b88c3db1ccfbc031d9752407bd9ca64d171c10 Mon Sep 17 00:00:00 2001 From: Paulo Margarido <64600052+paulomarg@users.noreply.github.com> Date: Fri, 8 Dec 2023 14:19:51 -0500 Subject: [PATCH 2/2] Temporarily removing link so check passes --- .changeset/rich-ladybugs-own.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/.changeset/rich-ladybugs-own.md b/.changeset/rich-ladybugs-own.md index 6ed4cb696..2279848ca 100644 --- a/.changeset/rich-ladybugs-own.md +++ b/.changeset/rich-ladybugs-own.md @@ -9,5 +9,3 @@ 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. - -For more information and examples, see the [migration guide to v9](/packages/shopify-api/docs/migrating-to-v9.md#changes-to-runtime-adapters).