From 2b0859344115b07b9ba183653c4c9d3f51696749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dcaro=20Azevedo?= Date: Wed, 9 Feb 2022 13:58:36 -0300 Subject: [PATCH 1/6] fix: wrong allCollections query pagination and pagetype timeout errors --- packages/api/package.json | 3 +- .../vtex/clients/commerce/types/Portal.ts | 21 +++++++----- .../api/src/platforms/vtex/loaders/index.ts | 3 ++ .../src/platforms/vtex/loaders/pagetype.ts | 26 ++++++++++++++ .../platforms/vtex/resolvers/collection.ts | 26 +++++++------- .../api/src/platforms/vtex/resolvers/query.ts | 28 +++++++++------ packages/gatsby-source-store/src/paginate.ts | 11 +++--- yarn.lock | 34 ++++++++++++++++++- 8 files changed, 114 insertions(+), 38 deletions(-) create mode 100644 packages/api/src/platforms/vtex/loaders/pagetype.ts diff --git a/packages/api/package.json b/packages/api/package.json index 90e7a17c54..f379bee6a6 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -26,7 +26,8 @@ "@sindresorhus/slugify": "^1.1.2", "dataloader": "^2.0.0", "fast-deep-equal": "^3.1.3", - "isomorphic-unfetch": "^3.1.0" + "isomorphic-unfetch": "^3.1.0", + "p-limit": "^4.0.0" }, "devDependencies": { "@graphql-codegen/cli": "2.2.0", diff --git a/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts b/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts index 2a589413ce..94d56b8276 100644 --- a/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts +++ b/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts @@ -1,14 +1,19 @@ -export interface PortalPagetype { +export type PortalPagetype = ValidPortalPagetype | NotFoundPortalPagetype + +export interface ValidPortalPagetype { id: number name: string url: string title: string metaTagDescription: string - pageType: - | 'Brand' - | 'Category' - | 'Department' - | 'Subcategory' - | 'FullText' - | 'NotFound' + pageType: 'Brand' | 'Category' | 'Department' | 'Subcategory' | 'FullText' +} + +export interface NotFoundPortalPagetype { + id: null + name: null + url: null + title: null + metaTagDescription: null + pageType: 'NotFound' } diff --git a/packages/api/src/platforms/vtex/loaders/index.ts b/packages/api/src/platforms/vtex/loaders/index.ts index aa86ea268a..2e08b9c7da 100644 --- a/packages/api/src/platforms/vtex/loaders/index.ts +++ b/packages/api/src/platforms/vtex/loaders/index.ts @@ -1,5 +1,6 @@ import { getSimulationLoader } from './simulation' import { getSkuLoader } from './sku' +import { getPagetypeLoader } from './pagetype' import type { Context, Options } from '..' export type Loaders = ReturnType @@ -7,9 +8,11 @@ export type Loaders = ReturnType export const getLoaders = (options: Options, { clients }: Context) => { const skuLoader = getSkuLoader(options, clients) const simulationLoader = getSimulationLoader(options, clients) + const pagetypeLoader = getPagetypeLoader(options, clients) return { skuLoader, simulationLoader, + pagetypeLoader, } } diff --git a/packages/api/src/platforms/vtex/loaders/pagetype.ts b/packages/api/src/platforms/vtex/loaders/pagetype.ts new file mode 100644 index 0000000000..46104d676c --- /dev/null +++ b/packages/api/src/platforms/vtex/loaders/pagetype.ts @@ -0,0 +1,26 @@ +import DataLoader from 'dataloader' +import pLimit from 'p-limit' + +import type { Options } from '..' +import type { Clients } from '../clients' +import type { PortalPagetype } from '../clients/commerce/types/Portal' + +// Limits concurrent requests to 20 so that they don't timeout +const CONCURRENT_REQUESTS_MAX = 20 + +export const getPagetypeLoader = (_: Options, clients: Clients) => { + const limit = pLimit(CONCURRENT_REQUESTS_MAX) + + const loader = (slugs: readonly string[]) => { + return Promise.all( + slugs.map((s: string) => + limit(() => clients.commerce.catalog.portal.pagetype(s)) + ) + ) + } + + return new DataLoader(loader, { + // DataLoader is being used to cache requests, not to batch them + batch: false, + }) +} diff --git a/packages/api/src/platforms/vtex/resolvers/collection.ts b/packages/api/src/platforms/vtex/resolvers/collection.ts index 3dfd585fce..7a86f89915 100644 --- a/packages/api/src/platforms/vtex/resolvers/collection.ts +++ b/packages/api/src/platforms/vtex/resolvers/collection.ts @@ -2,21 +2,21 @@ import { slugify as baseSlugify } from '../utils/slugify' import type { Resolver } from '..' import type { Brand } from '../clients/commerce/types/Brand' import type { CategoryTree } from '../clients/commerce/types/CategoryTree' -import type { PortalPagetype } from '../clients/commerce/types/Portal' +import type { ValidPortalPagetype } from '../clients/commerce/types/Portal' -type Root = Brand | (CategoryTree & { level: number }) | PortalPagetype +type Root = Brand | (CategoryTree & { level: number }) | ValidPortalPagetype const isBrand = (x: any): x is Brand => x.type === 'brand' -const isPortalPageType = (x: any): x is PortalPagetype => - typeof x.pageType === 'string' +const isValidPortalPageType = (x: any): x is ValidPortalPagetype => + typeof x.pageType === 'string' && x.pageType !== 'NotFound' const slugify = (root: Root) => { if (isBrand(root)) { return baseSlugify(root.name) } - if (isPortalPageType(root)) { + if (isValidPortalPageType(root)) { return new URL(`https://${root.url}`).pathname.slice(1) } @@ -27,7 +27,7 @@ export const StoreCollection: Record> = { id: ({ id }) => id.toString(), slug: (root) => slugify(root), seo: (root) => - isBrand(root) || isPortalPageType(root) + isBrand(root) || isValidPortalPageType(root) ? { title: root.title, description: root.metaTagDescription, @@ -39,7 +39,7 @@ export const StoreCollection: Record> = { type: (root) => isBrand(root) ? 'Brand' - : isPortalPageType(root) + : isValidPortalPageType(root) ? root.pageType : root.level === 0 ? 'Department' @@ -51,7 +51,7 @@ export const StoreCollection: Record> = { } : { selectedFacets: new URL( - isPortalPageType(root) ? `https://${root.url}` : root.url + isValidPortalPageType(root) ? `https://${root.url}` : root.url ).pathname .slice(1) .split('/') @@ -62,7 +62,7 @@ export const StoreCollection: Record> = { }, breadcrumbList: async (root, _, ctx) => { const { - clients: { commerce }, + loaders: { pagetypeLoader }, } = ctx const slug = slugify(root) @@ -78,17 +78,17 @@ export const StoreCollection: Record> = { segments.slice(0, index + 1).join('/') ) - const pageTypes = await Promise.all( - slugs.map((s) => commerce.catalog.portal.pagetype(s)) + const validPageTypes = (await pagetypeLoader.loadMany(slugs)).filter( + isValidPortalPageType ) return { - itemListElement: pageTypes.map((pageType, index) => ({ + itemListElement: validPageTypes.map((pageType, index) => ({ item: new URL(`https://${pageType.url}`).pathname.toLowerCase(), name: pageType.name, position: index + 1, })), - numberOfItems: pageTypes.length, + numberOfItems: validPageTypes.length, } }, } diff --git a/packages/api/src/platforms/vtex/resolvers/query.ts b/packages/api/src/platforms/vtex/resolvers/query.ts index bcff325964..e57c0be0c0 100644 --- a/packages/api/src/platforms/vtex/resolvers/query.ts +++ b/packages/api/src/platforms/vtex/resolvers/query.ts @@ -12,6 +12,7 @@ import type { } from '../../../__generated__/schema' import type { CategoryTree } from '../clients/commerce/types/CategoryTree' import type { Context } from '../index' +import type { ValidPortalPagetype } from '../clients/commerce/types/Portal' export const Query = { product: async (_: unknown, { locator }: QueryProductArgs, ctx: Context) => { @@ -35,15 +36,15 @@ export const Query = { ctx: Context ) => { const { - clients: { commerce }, + loaders: { pagetypeLoader }, } = ctx - const result = await commerce.catalog.portal.pagetype(slug) + const result = await pagetypeLoader.load(slug) const whitelist = ['Brand', 'Category', 'Department', 'Subcategory'] if (whitelist.includes(result.pageType)) { - return result + return result as ValidPortalPagetype } throw new NotFoundError(`Not Found: ${slug}`) @@ -100,6 +101,7 @@ export const Query = { endCursor: products.total.toString(), totalCount: products.total, }, + // after + index is bigger than after+first itself because of the array flat() above edges: skus.map((sku, index) => ({ node: sku, cursor: (after + index).toString(), @@ -140,20 +142,24 @@ export const Query = { ...categories, ] + const validCollections = collections + // Nullable slugs may cause one route to override the other + .filter((node) => Boolean(StoreCollection.slug(node, null, ctx, null))) + return { pageInfo: { - hasNextPage: false, - hasPreviousPage: false, + hasNextPage: validCollections.length - after > first, + hasPreviousPage: after > 0, startCursor: '0', - endCursor: '0', + endCursor: ( + Math.min(first, validCollections.length - after) - 1 + ).toString(), }, - edges: collections - // Nullable slugs may cause one route to override the other - .filter((node) => Boolean(StoreCollection.slug(node, null, ctx, null))) - .slice(after, first) + edges: validCollections + .slice(after, after + first) .map((node, index) => ({ node, - cursor: index.toString(), + cursor: (after + index).toString(), })), } }, diff --git a/packages/gatsby-source-store/src/paginate.ts b/packages/gatsby-source-store/src/paginate.ts index e492768bf8..a995e6bed6 100644 --- a/packages/gatsby-source-store/src/paginate.ts +++ b/packages/gatsby-source-store/src/paginate.ts @@ -22,11 +22,14 @@ export const RelayForward = ( } }, next(state, page) { - const tail = page.edges[page.edges.length - 1] - const first = - Number(state.variables.first) ?? Math.min(DEFAULT_PAGE_SIZE, maxItems) + const { first: rawFirst, after: maybeStateAfter } = state.variables - const after = tail?.cursor + const stateAfter = Number.isNaN(Number(maybeStateAfter)) + ? 0 + : Number(maybeStateAfter) + + const first = Number(rawFirst) ?? DEFAULT_PAGE_SIZE + const after = (stateAfter + first).toString() return { variables: { first, after }, diff --git a/yarn.lock b/yarn.lock index a86f5b59a4..846b44b22a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12011,6 +12011,21 @@ chokidar@^3.5.2: optionalDependencies: fsevents "~2.3.2" +chokidar@^3.5.3: + version "3.5.3" + resolved "https://registry.yarnpkg.com/chokidar/-/chokidar-3.5.3.tgz#1cf37c8707b932bd1af1ae22c0432e2acd1903bd" + integrity sha512-Dr3sfKRP6oTcjf2JmUmFJfeVMvXBdegxB0iVQ5eb2V10uFJUCAS8OByZdVAyVb8xXNz3GjjTgj9kLWsZTqE6kw== + dependencies: + anymatch "~3.1.2" + braces "~3.0.2" + glob-parent "~5.1.2" + is-binary-path "~2.1.0" + is-glob "~4.0.1" + normalize-path "~3.0.0" + readdirp "~3.6.0" + optionalDependencies: + fsevents "~2.3.2" + chownr@^1.1.1, chownr@^1.1.2, chownr@^1.1.4: version "1.1.4" resolved "https://registry.yarnpkg.com/chownr/-/chownr-1.1.4.tgz#6fc9d7b42d32a583596337666e7d08084da2cc6b" @@ -23166,6 +23181,13 @@ p-limit@^2.0.0, p-limit@^2.2.0: dependencies: p-try "^2.0.0" +p-limit@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-4.0.0.tgz#914af6544ed32bfa54670b061cafcbd04984b644" + integrity sha512-5b0R4txpzjPWVw/cXXUResoD4hb6U/x9BH08L7nw+GN1sezDzPdxeRvpc9c433fZhBan/wusjbCsqwqm4EIBIQ== + dependencies: + yocto-queue "^1.0.0" + p-locate@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-2.0.0.tgz#20a0103b222a70c8fd39cc2e580680f3dde5ec43" @@ -30854,7 +30876,12 @@ ws@^7.3.0, ws@^7.3.1, ws@~7.4.2: resolved "https://registry.yarnpkg.com/ws/-/ws-7.4.5.tgz#a484dd851e9beb6fdb420027e3885e8ce48986c1" integrity sha512-xzyu3hFvomRfXKH8vOFMU3OguG6oOvhXMo3xsGy3xWExqaM2dxBbVxuD99O7m3ZUFMvvscsZDqxfgMaRr/Nr1g== -ws@^8.1.0, ws@^8.2.3: +ws@^8.1.0: + version "8.5.0" + resolved "https://registry.yarnpkg.com/ws/-/ws-8.5.0.tgz#bfb4be96600757fe5382de12c670dab984a1ed4f" + integrity sha512-BWX0SWVgLPzYwF8lTzEy1egjhS4S4OEAHfsO8o65WOVsrnSRGaSiUaa9e0ggGlkMTtBlmOpEXiie9RUcBO86qg== + +ws@^8.2.3: version "8.3.0" resolved "https://registry.yarnpkg.com/ws/-/ws-8.3.0.tgz#7185e252c8973a60d57170175ff55fdbd116070d" integrity sha512-Gs5EZtpqZzLvmIM59w4igITU57lrtYVFneaa434VROv4thzJyV6UjIL3D42lslWlI+D4KzLYnxSwtfuiO79sNw== @@ -31067,6 +31094,11 @@ yocto-queue@^0.1.0: resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== +yocto-queue@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-1.0.0.tgz#7f816433fb2cbc511ec8bf7d263c3b58a1a3c251" + integrity sha512-9bnSc/HEW2uRy67wc+T8UwauLuPJVn28jb+GtJY16iiKWyvmYJRXVT4UamsAEGQfPohgr2q4Tq0sQbQlxTfi1g== + yoga-layout-prebuilt@^1.9.6: version "1.10.0" resolved "https://registry.yarnpkg.com/yoga-layout-prebuilt/-/yoga-layout-prebuilt-1.10.0.tgz#2936fbaf4b3628ee0b3e3b1df44936d6c146faa6" From e2c1f69447c1f591ef0b9fb18536dd95891eae7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dcaro=20Azevedo?= Date: Thu, 10 Feb 2022 11:50:14 -0300 Subject: [PATCH 2/6] chore: rename 'whitelist' to 'allowList' --- packages/api/src/platforms/vtex/resolvers/query.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/src/platforms/vtex/resolvers/query.ts b/packages/api/src/platforms/vtex/resolvers/query.ts index e57c0be0c0..9e237c0682 100644 --- a/packages/api/src/platforms/vtex/resolvers/query.ts +++ b/packages/api/src/platforms/vtex/resolvers/query.ts @@ -41,9 +41,9 @@ export const Query = { const result = await pagetypeLoader.load(slug) - const whitelist = ['Brand', 'Category', 'Department', 'Subcategory'] + const allowList = ['Brand', 'Category', 'Department', 'Subcategory'] - if (whitelist.includes(result.pageType)) { + if (allowList.includes(result.pageType)) { return result as ValidPortalPagetype } From 8c1f17b172d2e96ad28b8b7f985d2b2c7cffd53e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dcaro=20Azevedo?= Date: Mon, 14 Feb 2022 17:35:20 -0300 Subject: [PATCH 3/6] chore: downgrade p-limit dependency --- packages/api/package.json | 2 +- yarn.lock | 27 --------------------------- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/packages/api/package.json b/packages/api/package.json index f379bee6a6..44291b4eac 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -27,7 +27,7 @@ "dataloader": "^2.0.0", "fast-deep-equal": "^3.1.3", "isomorphic-unfetch": "^3.1.0", - "p-limit": "^4.0.0" + "p-limit": "^3.1.0" }, "devDependencies": { "@graphql-codegen/cli": "2.2.0", diff --git a/yarn.lock b/yarn.lock index 846b44b22a..09ea68ab42 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12011,21 +12011,6 @@ chokidar@^3.5.2: optionalDependencies: fsevents "~2.3.2" -chokidar@^3.5.3: - version "3.5.3" - resolved "https://registry.yarnpkg.com/chokidar/-/chokidar-3.5.3.tgz#1cf37c8707b932bd1af1ae22c0432e2acd1903bd" - integrity sha512-Dr3sfKRP6oTcjf2JmUmFJfeVMvXBdegxB0iVQ5eb2V10uFJUCAS8OByZdVAyVb8xXNz3GjjTgj9kLWsZTqE6kw== - dependencies: - anymatch "~3.1.2" - braces "~3.0.2" - glob-parent "~5.1.2" - is-binary-path "~2.1.0" - is-glob "~4.0.1" - normalize-path "~3.0.0" - readdirp "~3.6.0" - optionalDependencies: - fsevents "~2.3.2" - chownr@^1.1.1, chownr@^1.1.2, chownr@^1.1.4: version "1.1.4" resolved "https://registry.yarnpkg.com/chownr/-/chownr-1.1.4.tgz#6fc9d7b42d32a583596337666e7d08084da2cc6b" @@ -23181,13 +23166,6 @@ p-limit@^2.0.0, p-limit@^2.2.0: dependencies: p-try "^2.0.0" -p-limit@^4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-4.0.0.tgz#914af6544ed32bfa54670b061cafcbd04984b644" - integrity sha512-5b0R4txpzjPWVw/cXXUResoD4hb6U/x9BH08L7nw+GN1sezDzPdxeRvpc9c433fZhBan/wusjbCsqwqm4EIBIQ== - dependencies: - yocto-queue "^1.0.0" - p-locate@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-2.0.0.tgz#20a0103b222a70c8fd39cc2e580680f3dde5ec43" @@ -31094,11 +31072,6 @@ yocto-queue@^0.1.0: resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== -yocto-queue@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-1.0.0.tgz#7f816433fb2cbc511ec8bf7d263c3b58a1a3c251" - integrity sha512-9bnSc/HEW2uRy67wc+T8UwauLuPJVn28jb+GtJY16iiKWyvmYJRXVT4UamsAEGQfPohgr2q4Tq0sQbQlxTfi1g== - yoga-layout-prebuilt@^1.9.6: version "1.10.0" resolved "https://registry.yarnpkg.com/yoga-layout-prebuilt/-/yoga-layout-prebuilt-1.10.0.tgz#2936fbaf4b3628ee0b3e3b1df44936d6c146faa6" From f613efcae10fc6059d0bf096df86948d2be7499c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dcaro=20Azevedo?= Date: Thu, 17 Feb 2022 16:04:06 -0300 Subject: [PATCH 4/6] fix: removed NotFound pageType filter and added comprehensive error message --- .../src/platforms/vtex/loaders/pagetype.ts | 58 ++++++++++++++++++- .../platforms/vtex/resolvers/collection.ts | 17 ++++-- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/packages/api/src/platforms/vtex/loaders/pagetype.ts b/packages/api/src/platforms/vtex/loaders/pagetype.ts index 46104d676c..afc3a10ff3 100644 --- a/packages/api/src/platforms/vtex/loaders/pagetype.ts +++ b/packages/api/src/platforms/vtex/loaders/pagetype.ts @@ -3,11 +3,67 @@ import pLimit from 'p-limit' import type { Options } from '..' import type { Clients } from '../clients' -import type { PortalPagetype } from '../clients/commerce/types/Portal' +import type { + PortalPagetype, + ValidPortalPagetype, +} from '../clients/commerce/types/Portal' +import { isValidPortalPageType } from '../resolvers/collection' // Limits concurrent requests to 20 so that they don't timeout const CONCURRENT_REQUESTS_MAX = 20 +export function throwIfPageTypeNotFound( + pageTypes: PromiseSettledResult[], + slugs: readonly string[] +): pageTypes is PromiseFulfilledResult[] { + const notFoundIndexes: number[] = [] + const rejectedIndexes: number[] = [] + + pageTypes.forEach((pageType, index) => { + // If the request fails + if (pageType.status === 'rejected') { + rejectedIndexes.push(index) + + return + } + + // If the catalog returns NotFound pageType + if (!isValidPortalPageType(pageType.value)) { + notFoundIndexes.push(index) + } + }) + + if (notFoundIndexes.length + rejectedIndexes.length > 0) { + let errorMessage = '' + + if (notFoundIndexes.length > 0) { + errorMessage += + `Catalog returned NotFound pageType for the following slug(s): ${notFoundIndexes + .map((i) => slugs[i]) + .join(', ')}.` + + ' This usually happens when there is more than one category with' + + ' the same name in the same category tree level.' + + errorMessage += '\n' + } + + if (rejectedIndexes.length > 0) { + errorMessage += `pageType request failed for the following slug(s): ${rejectedIndexes + .map((i) => slugs[i]) + .join(', ')}.` + + errorMessage += '\n' + errorMessage += rejectedIndexes + .map((i) => (pageTypes[i] as PromiseRejectedResult).reason) + .join(`\n`) + } + + throw new Error(errorMessage) + } + + return true +} + export const getPagetypeLoader = (_: Options, clients: Clients) => { const limit = pLimit(CONCURRENT_REQUESTS_MAX) diff --git a/packages/api/src/platforms/vtex/resolvers/collection.ts b/packages/api/src/platforms/vtex/resolvers/collection.ts index 7a86f89915..894c952f5d 100644 --- a/packages/api/src/platforms/vtex/resolvers/collection.ts +++ b/packages/api/src/platforms/vtex/resolvers/collection.ts @@ -3,12 +3,13 @@ import type { Resolver } from '..' import type { Brand } from '../clients/commerce/types/Brand' import type { CategoryTree } from '../clients/commerce/types/CategoryTree' import type { ValidPortalPagetype } from '../clients/commerce/types/Portal' +import { throwIfPageTypeNotFound } from '../loaders/pagetype' type Root = Brand | (CategoryTree & { level: number }) | ValidPortalPagetype const isBrand = (x: any): x is Brand => x.type === 'brand' -const isValidPortalPageType = (x: any): x is ValidPortalPagetype => +export const isValidPortalPageType = (x: any): x is ValidPortalPagetype => typeof x.pageType === 'string' && x.pageType !== 'NotFound' const slugify = (root: Root) => { @@ -78,17 +79,23 @@ export const StoreCollection: Record> = { segments.slice(0, index + 1).join('/') ) - const validPageTypes = (await pagetypeLoader.loadMany(slugs)).filter( - isValidPortalPageType + const pageTypePromises = await Promise.allSettled( + slugs.map((slugSegment) => pagetypeLoader.load(slugSegment)) ) + throwIfPageTypeNotFound(pageTypePromises, slugs) + + const pageTypes = (pageTypePromises as Array< + PromiseFulfilledResult + >).map((pageType) => pageType.value) + return { - itemListElement: validPageTypes.map((pageType, index) => ({ + itemListElement: pageTypes.map((pageType, index) => ({ item: new URL(`https://${pageType.url}`).pathname.toLowerCase(), name: pageType.name, position: index + 1, })), - numberOfItems: validPageTypes.length, + numberOfItems: pageTypes.length, } }, } From b54ec72722a7321652600043585700c15bd7116b Mon Sep 17 00:00:00 2001 From: Tiago Gimenes Date: Mon, 21 Feb 2022 11:46:33 -0300 Subject: [PATCH 5/6] rename types and pageTypeLoader to collectionLoader --- .../vtex/clients/commerce/types/Portal.ts | 14 ++-- .../src/platforms/vtex/loaders/collection.ts | 50 +++++++++++ .../api/src/platforms/vtex/loaders/index.ts | 6 +- .../src/platforms/vtex/loaders/pagetype.ts | 82 ------------------- .../platforms/vtex/resolvers/collection.ts | 51 ++++++------ .../api/src/platforms/vtex/resolvers/query.ts | 20 +---- 6 files changed, 87 insertions(+), 136 deletions(-) create mode 100644 packages/api/src/platforms/vtex/loaders/collection.ts delete mode 100644 packages/api/src/platforms/vtex/loaders/pagetype.ts diff --git a/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts b/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts index 94d56b8276..b6cee9ae6c 100644 --- a/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts +++ b/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts @@ -1,19 +1,19 @@ -export type PortalPagetype = ValidPortalPagetype | NotFoundPortalPagetype +export type PortalPagetype = CollectionPageType | FallbackPageType -export interface ValidPortalPagetype { +export interface CollectionPageType { id: number name: string url: string title: string metaTagDescription: string - pageType: 'Brand' | 'Category' | 'Department' | 'Subcategory' | 'FullText' + pageType: 'Brand' | 'Category' | 'Department' | 'Subcategory' } -export interface NotFoundPortalPagetype { +export interface FallbackPageType { id: null - name: null - url: null + name: null | string + url: null | string title: null metaTagDescription: null - pageType: 'NotFound' + pageType: 'NotFound' | 'FullText' } diff --git a/packages/api/src/platforms/vtex/loaders/collection.ts b/packages/api/src/platforms/vtex/loaders/collection.ts new file mode 100644 index 0000000000..a1c66d9d10 --- /dev/null +++ b/packages/api/src/platforms/vtex/loaders/collection.ts @@ -0,0 +1,50 @@ +import DataLoader from 'dataloader' +import pLimit from 'p-limit' + +import { NotFoundError } from '../utils/errors' +import type { CollectionPageType } from '../clients/commerce/types/Portal' +import type { Options } from '..' +import type { Clients } from '../clients' + +// Limits concurrent requests to 20 so that they don't timeout +const CONCURRENT_REQUESTS_MAX = 20 + +const collectionPageTypes = new Set([ + 'brand', + 'category', + 'department', + 'subcategory', +] as const) + +export const isCollectionPageType = (x: any): x is CollectionPageType => + typeof x.pageType === 'string' && + collectionPageTypes.has(x.pageType.toLowerCase()) + +export const getCollectionLoader = (_: Options, clients: Clients) => { + const limit = pLimit(CONCURRENT_REQUESTS_MAX) + + const loader = async ( + slugs: readonly string[] + ): Promise => { + return Promise.all( + slugs.map((slug: string) => + limit(async () => { + const page = await clients.commerce.catalog.portal.pagetype(slug) + + if (isCollectionPageType(page)) { + return page + } + + throw new NotFoundError( + `Catalog returned ${page.pageType} for slug: ${slug}. This usually happens when there is more than one category with the same name in the same category tree level.` + ) + }) + ) + ) + } + + return new DataLoader(loader, { + // DataLoader is being used to cache requests, not to batch them + batch: false, + }) +} diff --git a/packages/api/src/platforms/vtex/loaders/index.ts b/packages/api/src/platforms/vtex/loaders/index.ts index 2e08b9c7da..b0f73e81a0 100644 --- a/packages/api/src/platforms/vtex/loaders/index.ts +++ b/packages/api/src/platforms/vtex/loaders/index.ts @@ -1,6 +1,6 @@ import { getSimulationLoader } from './simulation' import { getSkuLoader } from './sku' -import { getPagetypeLoader } from './pagetype' +import { getCollectionLoader } from './collection' import type { Context, Options } from '..' export type Loaders = ReturnType @@ -8,11 +8,11 @@ export type Loaders = ReturnType export const getLoaders = (options: Options, { clients }: Context) => { const skuLoader = getSkuLoader(options, clients) const simulationLoader = getSimulationLoader(options, clients) - const pagetypeLoader = getPagetypeLoader(options, clients) + const collectionLoader = getCollectionLoader(options, clients) return { skuLoader, simulationLoader, - pagetypeLoader, + collectionLoader, } } diff --git a/packages/api/src/platforms/vtex/loaders/pagetype.ts b/packages/api/src/platforms/vtex/loaders/pagetype.ts deleted file mode 100644 index afc3a10ff3..0000000000 --- a/packages/api/src/platforms/vtex/loaders/pagetype.ts +++ /dev/null @@ -1,82 +0,0 @@ -import DataLoader from 'dataloader' -import pLimit from 'p-limit' - -import type { Options } from '..' -import type { Clients } from '../clients' -import type { - PortalPagetype, - ValidPortalPagetype, -} from '../clients/commerce/types/Portal' -import { isValidPortalPageType } from '../resolvers/collection' - -// Limits concurrent requests to 20 so that they don't timeout -const CONCURRENT_REQUESTS_MAX = 20 - -export function throwIfPageTypeNotFound( - pageTypes: PromiseSettledResult[], - slugs: readonly string[] -): pageTypes is PromiseFulfilledResult[] { - const notFoundIndexes: number[] = [] - const rejectedIndexes: number[] = [] - - pageTypes.forEach((pageType, index) => { - // If the request fails - if (pageType.status === 'rejected') { - rejectedIndexes.push(index) - - return - } - - // If the catalog returns NotFound pageType - if (!isValidPortalPageType(pageType.value)) { - notFoundIndexes.push(index) - } - }) - - if (notFoundIndexes.length + rejectedIndexes.length > 0) { - let errorMessage = '' - - if (notFoundIndexes.length > 0) { - errorMessage += - `Catalog returned NotFound pageType for the following slug(s): ${notFoundIndexes - .map((i) => slugs[i]) - .join(', ')}.` + - ' This usually happens when there is more than one category with' + - ' the same name in the same category tree level.' - - errorMessage += '\n' - } - - if (rejectedIndexes.length > 0) { - errorMessage += `pageType request failed for the following slug(s): ${rejectedIndexes - .map((i) => slugs[i]) - .join(', ')}.` - - errorMessage += '\n' - errorMessage += rejectedIndexes - .map((i) => (pageTypes[i] as PromiseRejectedResult).reason) - .join(`\n`) - } - - throw new Error(errorMessage) - } - - return true -} - -export const getPagetypeLoader = (_: Options, clients: Clients) => { - const limit = pLimit(CONCURRENT_REQUESTS_MAX) - - const loader = (slugs: readonly string[]) => { - return Promise.all( - slugs.map((s: string) => - limit(() => clients.commerce.catalog.portal.pagetype(s)) - ) - ) - } - - return new DataLoader(loader, { - // DataLoader is being used to cache requests, not to batch them - batch: false, - }) -} diff --git a/packages/api/src/platforms/vtex/resolvers/collection.ts b/packages/api/src/platforms/vtex/resolvers/collection.ts index 894c952f5d..90627ba180 100644 --- a/packages/api/src/platforms/vtex/resolvers/collection.ts +++ b/packages/api/src/platforms/vtex/resolvers/collection.ts @@ -1,23 +1,20 @@ +import { isCollectionPageType } from '../loaders/collection' import { slugify as baseSlugify } from '../utils/slugify' import type { Resolver } from '..' import type { Brand } from '../clients/commerce/types/Brand' import type { CategoryTree } from '../clients/commerce/types/CategoryTree' -import type { ValidPortalPagetype } from '../clients/commerce/types/Portal' -import { throwIfPageTypeNotFound } from '../loaders/pagetype' +import type { CollectionPageType } from '../clients/commerce/types/Portal' -type Root = Brand | (CategoryTree & { level: number }) | ValidPortalPagetype +type Root = Brand | (CategoryTree & { level: number }) | CollectionPageType const isBrand = (x: any): x is Brand => x.type === 'brand' -export const isValidPortalPageType = (x: any): x is ValidPortalPagetype => - typeof x.pageType === 'string' && x.pageType !== 'NotFound' - const slugify = (root: Root) => { if (isBrand(root)) { - return baseSlugify(root.name) + return baseSlugify(root.name.toLowerCase()) } - if (isValidPortalPageType(root)) { + if (isCollectionPageType(root)) { return new URL(`https://${root.url}`).pathname.slice(1) } @@ -28,7 +25,7 @@ export const StoreCollection: Record> = { id: ({ id }) => id.toString(), slug: (root) => slugify(root), seo: (root) => - isBrand(root) || isValidPortalPageType(root) + isBrand(root) || isCollectionPageType(root) ? { title: root.title, description: root.metaTagDescription, @@ -40,7 +37,7 @@ export const StoreCollection: Record> = { type: (root) => isBrand(root) ? 'Brand' - : isValidPortalPageType(root) + : isCollectionPageType(root) ? root.pageType : root.level === 0 ? 'Department' @@ -52,7 +49,7 @@ export const StoreCollection: Record> = { } : { selectedFacets: new URL( - isValidPortalPageType(root) ? `https://${root.url}` : root.url + isCollectionPageType(root) ? `https://${root.url}` : root.url ).pathname .slice(1) .split('/') @@ -63,7 +60,7 @@ export const StoreCollection: Record> = { }, breadcrumbList: async (root, _, ctx) => { const { - loaders: { pagetypeLoader }, + loaders: { collectionLoader }, } = ctx const slug = slugify(root) @@ -79,23 +76,23 @@ export const StoreCollection: Record> = { segments.slice(0, index + 1).join('/') ) - const pageTypePromises = await Promise.allSettled( - slugs.map((slugSegment) => pagetypeLoader.load(slugSegment)) - ) - - throwIfPageTypeNotFound(pageTypePromises, slugs) + try { + const collections = await Promise.all( + slugs.map((s) => collectionLoader.load(s)) + ) - const pageTypes = (pageTypePromises as Array< - PromiseFulfilledResult - >).map((pageType) => pageType.value) + return { + itemListElement: collections.map((collection, index) => ({ + item: new URL(`https://${collection.url}`).pathname.toLowerCase(), + name: collection.name, + position: index + 1, + })), + numberOfItems: collections.length, + } + } catch (err) { + console.error({ slug, root, segments, slugs }) - return { - itemListElement: pageTypes.map((pageType, index) => ({ - item: new URL(`https://${pageType.url}`).pathname.toLowerCase(), - name: pageType.name, - position: index + 1, - })), - numberOfItems: pageTypes.length, + throw err } }, } diff --git a/packages/api/src/platforms/vtex/resolvers/query.ts b/packages/api/src/platforms/vtex/resolvers/query.ts index 9e237c0682..ecaef20f3a 100644 --- a/packages/api/src/platforms/vtex/resolvers/query.ts +++ b/packages/api/src/platforms/vtex/resolvers/query.ts @@ -1,5 +1,4 @@ import { enhanceSku } from '../utils/enhanceSku' -import { NotFoundError } from '../utils/errors' import { transformSelectedFacet } from '../utils/facets' import { SORT_MAP } from '../utils/sort' import { StoreCollection } from './collection' @@ -12,7 +11,6 @@ import type { } from '../../../__generated__/schema' import type { CategoryTree } from '../clients/commerce/types/CategoryTree' import type { Context } from '../index' -import type { ValidPortalPagetype } from '../clients/commerce/types/Portal' export const Query = { product: async (_: unknown, { locator }: QueryProductArgs, ctx: Context) => { @@ -30,24 +28,12 @@ export const Query = { return skuLoader.load(locator.map(transformSelectedFacet)) }, - collection: async ( - _: unknown, - { slug }: QueryCollectionArgs, - ctx: Context - ) => { + collection: (_: unknown, { slug }: QueryCollectionArgs, ctx: Context) => { const { - loaders: { pagetypeLoader }, + loaders: { collectionLoader }, } = ctx - const result = await pagetypeLoader.load(slug) - - const allowList = ['Brand', 'Category', 'Department', 'Subcategory'] - - if (allowList.includes(result.pageType)) { - return result as ValidPortalPagetype - } - - throw new NotFoundError(`Not Found: ${slug}`) + return collectionLoader.load(slug) }, search: async ( _: unknown, From 6e18a245a91a2f7d6dc68f18e7bd4cc0aceb8432 Mon Sep 17 00:00:00 2001 From: Tiago Gimenes Date: Mon, 21 Feb 2022 11:48:41 -0300 Subject: [PATCH 6/6] remove unecessary logs --- .../platforms/vtex/resolvers/collection.ts | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/api/src/platforms/vtex/resolvers/collection.ts b/packages/api/src/platforms/vtex/resolvers/collection.ts index 90627ba180..537156038d 100644 --- a/packages/api/src/platforms/vtex/resolvers/collection.ts +++ b/packages/api/src/platforms/vtex/resolvers/collection.ts @@ -76,23 +76,17 @@ export const StoreCollection: Record> = { segments.slice(0, index + 1).join('/') ) - try { - const collections = await Promise.all( - slugs.map((s) => collectionLoader.load(s)) - ) - - return { - itemListElement: collections.map((collection, index) => ({ - item: new URL(`https://${collection.url}`).pathname.toLowerCase(), - name: collection.name, - position: index + 1, - })), - numberOfItems: collections.length, - } - } catch (err) { - console.error({ slug, root, segments, slugs }) + const collections = await Promise.all( + slugs.map((s) => collectionLoader.load(s)) + ) - throw err + return { + itemListElement: collections.map((collection, index) => ({ + item: new URL(`https://${collection.url}`).pathname.toLowerCase(), + name: collection.name, + position: index + 1, + })), + numberOfItems: collections.length, } }, }