From e8ddf9de7c92325850c938cfeb219d0561885668 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 20 Mar 2023 16:09:22 +1100 Subject: [PATCH 1/2] Update `refs.test` to TS --- code/lib/manager-api/src/index.tsx | 3 +- .../src/tests/{refs.test.js => refs.test.ts} | 264 ++++++++++++++---- 2 files changed, 214 insertions(+), 53 deletions(-) rename code/lib/manager-api/src/tests/{refs.test.js => refs.test.ts} (78%) diff --git a/code/lib/manager-api/src/index.tsx b/code/lib/manager-api/src/index.tsx index 6822c39e9769..baa84f691776 100644 --- a/code/lib/manager-api/src/index.tsx +++ b/code/lib/manager-api/src/index.tsx @@ -164,7 +164,8 @@ type ModuleWithoutInit = Omit< >; export type ModuleFn = ( - m: ModuleArgs + m: ModuleArgs, + options?: any ) => HasInit extends true ? ModuleWithInit : ModuleWithoutInit; diff --git a/code/lib/manager-api/src/tests/refs.test.js b/code/lib/manager-api/src/tests/refs.test.ts similarity index 78% rename from code/lib/manager-api/src/tests/refs.test.js rename to code/lib/manager-api/src/tests/refs.test.ts index d2ddd297e397..266ec713a1fa 100644 --- a/code/lib/manager-api/src/tests/refs.test.js +++ b/code/lib/manager-api/src/tests/refs.test.ts @@ -3,6 +3,8 @@ import { getSourceType, init as initRefs } from '../modules/refs'; const { fetch } = global; +const fetchMock = jest.mocked(fetch); + jest.mock('@storybook/global', () => { const globalMock = { fetch: jest.fn(() => Promise.resolve({})), @@ -46,9 +48,33 @@ const store = { }, }, }), - setState: jest.fn(() => {}), + setState: jest.fn((a: any) => {}), }; +interface ResponseResult { + ok?: boolean; + err?: Error; + response?: () => never | object | Promise; +} + +type ResponseKeys = + | 'indexPrivate' + | 'indexPublic' + | 'storiesPrivate' + | 'storiesPublic' + | 'iframe' + | 'metadata'; + +function respond(result: ResponseResult): Promise { + const { err, ok, response } = result; + if (err) Promise.reject(err); + + return Promise.resolve({ + ok: ok ?? !!response, + json: response, + } as Response); +} + const setupResponses = ({ indexPrivate, indexPublic, @@ -56,46 +82,32 @@ const setupResponses = ({ storiesPublic, iframe, metadata, -}) => { - fetch.mockClear(); +}: Partial>) => { + fetchMock.mockClear(); store.setState.mockClear(); - fetch.mockImplementation((l, o) => { + fetchMock.mockImplementation((l, o) => { + if (typeof l !== 'string') { + throw new Error('Wrong request type'); + } + if (l.includes('index') && o.credentials === 'include' && indexPrivate) { - return Promise.resolve({ - ok: indexPrivate.ok, - json: indexPrivate.response, - }); + return respond(indexPrivate); } if (l.includes('index') && o.credentials === 'omit' && indexPublic) { - return Promise.resolve({ - ok: indexPublic.ok, - json: indexPublic.response, - }); + return respond(indexPublic); } if (l.includes('stories') && o.credentials === 'include' && storiesPrivate) { - return Promise.resolve({ - ok: storiesPrivate.ok, - json: storiesPrivate.response, - }); + return respond(storiesPrivate); } if (l.includes('stories') && o.credentials === 'omit' && storiesPublic) { - return Promise.resolve({ - ok: storiesPublic.ok, - json: storiesPublic.response, - }); + return respond(storiesPublic); } if (l.includes('iframe') && iframe) { - return Promise.resolve({ - ok: iframe.ok, - json: iframe.response, - }); + return respond(iframe); } if (l.includes('metadata') && metadata) { - return Promise.resolve({ - ok: metadata.ok, - json: metadata.response, - }); + return respond(metadata); } throw new Error(`Called URL ${l} without setting up mock`); }); @@ -138,9 +150,9 @@ describe('Refs API', () => { describe('checkRef', () => { it('on initialization it checks refs', async () => { // given - initRefs({ provider, store }); + initRefs({ provider, store } as any); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -174,9 +186,9 @@ describe('Refs API', () => { version: '2.1.3-rc.2', }, }; - initRefs({ provider, store }); + initRefs({ provider, store } as any); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json?version=2.1.3-rc.2", @@ -202,7 +214,155 @@ describe('Refs API', () => { it('checks refs (all fail)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); + + setupResponses({ + indexPrivate: { + ok: false, + response: async () => { + throw new Error('Failed to fetch'); + }, + }, + storiesPrivate: { + ok: false, + response: async () => { + throw new Error('Failed to fetch'); + }, + }, + }); + + await api.checkRef({ + id: 'fake', + url: 'https://example.com', + title: 'Fake', + }); + + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "https://example.com/index.json", + Object { + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], + Array [ + "https://example.com/stories.json", + Object { + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], + ] + `); + + expect(store.setState.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "refs": Object { + "fake": Object { + "id": "fake", + "index": undefined, + "indexError": Object { + "message": "Error: Loading of ref failed + at fetch (lib/api/src/modules/refs.ts) + + URL: https://example.com + + We weren't able to load the above URL, + it's possible a CORS error happened. + + Please check your dev-tools network tab.", + }, + "title": "Fake", + "type": "auto-inject", + "url": "https://example.com", + }, + }, + } + `); + }); + + it('checks refs (all 404)', async () => { + // given + const { api } = initRefs({ provider, store } as any, { runCheck: false }); + + setupResponses({ + indexPrivate: { + ok: false, + response: async () => { + throw new Error('Failed to fetch'); + }, + }, + storiesPrivate: { + ok: false, + response: async () => { + throw new Error('Failed to fetch'); + }, + }, + }); + + await api.checkRef({ + id: 'fake', + url: 'https://example.com', + title: 'Fake', + }); + + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "https://example.com/index.json", + Object { + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], + Array [ + "https://example.com/stories.json", + Object { + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], + ] + `); + + expect(store.setState.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "refs": Object { + "fake": Object { + "id": "fake", + "index": undefined, + "indexError": Object { + "message": "Error: Loading of ref failed + at fetch (lib/api/src/modules/refs.ts) + + URL: https://example.com + + We weren't able to load the above URL, + it's possible a CORS error happened. + + Please check your dev-tools network tab.", + }, + "title": "Fake", + "type": "auto-inject", + "url": "https://example.com", + }, + }, + } + `); + }); + + it('checks refs (index 404)', async () => { + // given + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { @@ -225,7 +385,7 @@ describe('Refs API', () => { title: 'Fake', }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -276,7 +436,7 @@ describe('Refs API', () => { it('checks refs (success)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { @@ -301,7 +461,7 @@ describe('Refs API', () => { title: 'Fake', }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -352,7 +512,7 @@ describe('Refs API', () => { it('checks refs (not replace versions)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { @@ -378,7 +538,7 @@ describe('Refs API', () => { versions: { a: 'http://example.com/a', b: 'http://example.com/b' }, }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -432,7 +592,7 @@ describe('Refs API', () => { it('checks refs (auth)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { @@ -455,7 +615,7 @@ describe('Refs API', () => { title: 'Fake', }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -506,7 +666,7 @@ describe('Refs API', () => { it('checks refs (basic-auth)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { @@ -529,7 +689,7 @@ describe('Refs API', () => { title: 'Fake', }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -568,9 +728,9 @@ describe('Refs API', () => { it('checks refs (mixed)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); - fetch.mockClear(); + fetchMock.mockClear(); store.setState.mockClear(); setupResponses({ @@ -596,7 +756,7 @@ describe('Refs API', () => { title: 'Fake', }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -651,7 +811,7 @@ describe('Refs API', () => { it('checks refs (serverside-success)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPublic: { @@ -677,7 +837,7 @@ describe('Refs API', () => { type: 'server-checked', }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -728,7 +888,7 @@ describe('Refs API', () => { it('checks refs (serverside-fail)', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { @@ -754,7 +914,7 @@ describe('Refs API', () => { type: 'unknown', }); - expect(fetch.mock.calls).toMatchInlineSnapshot(` + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` Array [ Array [ "https://example.com/index.json", @@ -806,7 +966,7 @@ describe('Refs API', () => { describe('v3 compatibility', () => { it('infers docs only if there is only one story and it has the name "Page"', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); const index = { v: 3, @@ -878,7 +1038,7 @@ describe('Refs API', () => { }); it('prefers parameters.docsOnly to inferred docsOnly status', async () => { - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); const index = { v: 3, @@ -926,7 +1086,7 @@ describe('Refs API', () => { it('errors on unknown version', async () => { // given - const { api } = initRefs({ provider, store }, { runCheck: false }); + const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { From 43d45fd261ca60ce5098f7016e367d3a8f2f69fd Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 20 Mar 2023 20:32:46 +1100 Subject: [PATCH 2/2] Deal with responses that throw in refs --- code/lib/manager-api/src/modules/refs.ts | 46 ++++--- code/lib/manager-api/src/tests/refs.test.ts | 130 +++++++++++++++----- 2 files changed, 121 insertions(+), 55 deletions(-) diff --git a/code/lib/manager-api/src/modules/refs.ts b/code/lib/manager-api/src/modules/refs.ts index c56f08fed9e3..328c7c0ddcaf 100644 --- a/code/lib/manager-api/src/modules/refs.ts +++ b/code/lib/manager-api/src/modules/refs.ts @@ -36,7 +36,7 @@ export interface SubAPI { changeRefState: (id: string, previewInitialized: boolean) => void; } -export const getSourceType = (source: string, refId: string) => { +export const getSourceType = (source: string, refId?: string) => { const { origin: localOrigin, pathname: localPathname } = location; const { origin: sourceOrigin, pathname: sourcePathname } = new URL(source); @@ -69,12 +69,8 @@ async function handleRequest( try { const response = await request; - if (response === false || response === true) { - return {}; - } - if (!response.ok) { - return {}; - } + if (response === false || response === true) throw new Error('Unexpected boolean response'); + if (!response.ok) throw new Error(`Unexpected response not OK: ${response.statusText}`); const json = await response.json(); @@ -179,28 +175,30 @@ export const init: ModuleFn = ( }); } - const [indexFetch, storiesFetch] = await Promise.all( + const [indexResult, storiesResult] = await Promise.all( ['index.json', 'stories.json'].map(async (file) => - fetch(`${urlParseResult.url}/${file}${query}`, { - headers, - credentials, - }) - ) - ); - - if (indexFetch.ok || storiesFetch.ok) { - const [index, metadata] = await Promise.all([ - indexFetch.ok ? handleRequest(indexFetch) : handleRequest(storiesFetch), handleRequest( - fetch(`${urlParseResult.url}/metadata.json${query}`, { + fetch(`${urlParseResult.url}/${file}${query}`, { headers, credentials, - cache: 'no-cache', - }).catch(() => false) - ), - ]); + }) + ) + ) + ); - Object.assign(loadedData, { ...index, ...metadata }); + if (!indexResult.indexError || !storiesResult.indexError) { + const metadata = await handleRequest( + fetch(`${urlParseResult.url}/metadata.json${query}`, { + headers, + credentials, + cache: 'no-cache', + }).catch(() => false) + ); + + Object.assign(loadedData, { + ...(indexResult.indexError ? storiesResult : indexResult), + ...(!metadata.indexError && metadata), + }); } else if (!isPublic) { // In theory the `/iframe.html` could be private and the `stories.json` could not exist, but in practice // the only private servers we know about (Chromatic) always include `stories.json`. So we can tell diff --git a/code/lib/manager-api/src/tests/refs.test.ts b/code/lib/manager-api/src/tests/refs.test.ts index 266ec713a1fa..f0556d560ba3 100644 --- a/code/lib/manager-api/src/tests/refs.test.ts +++ b/code/lib/manager-api/src/tests/refs.test.ts @@ -67,7 +67,9 @@ type ResponseKeys = function respond(result: ResponseResult): Promise { const { err, ok, response } = result; - if (err) Promise.reject(err); + if (err) { + return Promise.reject(err); + } return Promise.resolve({ ok: ok ?? !!response, @@ -286,22 +288,16 @@ describe('Refs API', () => { `); }); - it('checks refs (all 404)', async () => { + it('checks refs (all throw)', async () => { // given const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { - ok: false, - response: async () => { - throw new Error('Failed to fetch'); - }, + err: new Error('TypeError: Failed to fetch'), }, storiesPrivate: { - ok: false, - response: async () => { - throw new Error('Failed to fetch'); - }, + err: new Error('TypeError: Failed to fetch'), }, }); @@ -360,22 +356,23 @@ describe('Refs API', () => { `); }); - it('checks refs (index 404)', async () => { + it('checks refs (index throws)', async () => { // given const { api } = initRefs({ provider, store } as any, { runCheck: false }); setupResponses({ indexPrivate: { - ok: false, - response: async () => { - throw new Error('Failed to fetch'); - }, + err: new Error('TypeError: Failed to fetch'), }, storiesPrivate: { - ok: false, - response: async () => { - throw new Error('Failed to fetch'); - }, + ok: true, + response: async () => ({ v: 3, stories: {} }), + }, + metadata: { + ok: true, + response: async () => ({ + versions: {}, + }), }, }); @@ -405,6 +402,74 @@ describe('Refs API', () => { }, }, ], + Array [ + "https://example.com/metadata.json", + Object { + "cache": "no-cache", + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], + ] + `); + }); + + it('checks refs (metadata throws)', async () => { + // given + const { api } = initRefs({ provider, store } as any, { runCheck: false }); + + setupResponses({ + indexPrivate: { + ok: true, + response: async () => ({ v: 4, entries: {} }), + }, + storiesPrivate: { + ok: true, + response: async () => ({ v: 3, stories: {} }), + }, + metadata: { + err: new Error('TypeError: Failed to fetch'), + }, + }); + + await api.checkRef({ + id: 'fake', + url: 'https://example.com', + title: 'Fake', + }); + + expect(fetchMock.mock.calls).toMatchInlineSnapshot(` + Array [ + Array [ + "https://example.com/index.json", + Object { + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], + Array [ + "https://example.com/stories.json", + Object { + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], + Array [ + "https://example.com/metadata.json", + Object { + "cache": "no-cache", + "credentials": "include", + "headers": Object { + "Accept": "application/json", + }, + }, + ], ] `); @@ -413,20 +478,23 @@ describe('Refs API', () => { "refs": Object { "fake": Object { "id": "fake", - "index": undefined, - "indexError": Object { - "message": "Error: Loading of ref failed - at fetch (lib/api/src/modules/refs.ts) - - URL: https://example.com - - We weren't able to load the above URL, - it's possible a CORS error happened. + "index": Object {}, + "title": "Fake", + "type": "lazy", + "url": "https://example.com", + }, + }, + } + `); - Please check your dev-tools network tab.", - }, + expect(store.setState.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "refs": Object { + "fake": Object { + "id": "fake", + "index": Object {}, "title": "Fake", - "type": "auto-inject", + "type": "lazy", "url": "https://example.com", }, },