fix(core): Optimize collection variants N+1 and fix ms type issues

RubenDarioGuerreroNeira · RubenDarioGuerreroNeira · commit fac3fb789344 · 2026-04-10T18:45:43.000-04:00
diff --git a/packages/core/src/api/middleware/auth-guard.ts b/packages/core/src/api/middleware/auth-guard.ts
@@ -3,7 +3,7 @@ import { Reflector } from '@nestjs/core';
 import { Permission } from '@vendure/common/lib/generated-types';
 import { Request, Response } from 'express';
 import { GraphQLResolveInfo } from 'graphql';
-import ms, { type StringValue } from 'ms';
+import ms from 'ms';
 
 import { ForbiddenError } from '../../common/error/errors';
 import { API_KEY_AUTH_STRATEGY_NAME } from '../../config';
@@ -212,7 +212,7 @@ export class AuthGuard implements CanActivate {
         const lastUsedThreshold = new Date(
             Date.now() -
                 (typeof strategy.lastUsedAtUpdateInterval === 'string'
-                    ? ms(strategy.lastUsedAtUpdateInterval as StringValue)
+                    ? ms(strategy.lastUsedAtUpdateInterval)
                     : strategy.lastUsedAtUpdateInterval),
         );
         if (!apiKey.lastUsedAt || apiKey.lastUsedAt < lastUsedThreshold) {
diff --git a/packages/core/src/api/resolvers/admin/collection.resolver.ts b/packages/core/src/api/resolvers/admin/collection.resolver.ts
@@ -71,10 +71,11 @@ export class CollectionResolver {
             const countsPromise = this.collectionService.getProductVariantCounts(ctx, collectionIds);
             this.requestContextCache.set(ctx, CacheKey.CollectionVariantCounts, countsPromise);
         }
-        if (isFieldInSelection(info, 'variants')) {
+        if (isFieldInSelection(info, 'productVariants')) {
             const variantsPromise = this.collectionService.getProductVariantsForCollections(
                 ctx,
                 collectionIds,
+                ['taxCategory'],
             );
             this.requestContextCache.set(ctx, CacheKey.CollectionVariants, variantsPromise);
         }
diff --git a/packages/core/src/api/resolvers/entity/collection-entity.resolver.ts b/packages/core/src/api/resolvers/entity/collection-entity.resolver.ts
@@ -12,6 +12,7 @@ import { CacheKey } from '../../../common/constants';
 import { ListQueryOptions } from '../../../common/types/common-types';
 import { Translated } from '../../../common/types/locale-types';
 import { CollectionFilter } from '../../../config/catalog/collection-filter';
+import { ConfigService } from '../../../config/config.service';
 import { Asset, Collection, Product, ProductVariant } from '../../../entity';
 import { LocaleStringHydrator } from '../../../service/helpers/locale-string-hydrator/locale-string-hydrator';
 import { AssetService } from '../../../service/services/asset.service';
@@ -33,6 +34,7 @@ export class CollectionEntityResolver {
         private localeStringHydrator: LocaleStringHydrator,
         private configurableOperationCodec: ConfigurableOperationCodec,
         private requestContextCache: RequestContextCacheService,
+        private configService: ConfigService,
     ) {}
 
     @ResolveField()
@@ -72,14 +74,26 @@ export class CollectionEntityResolver {
                 const variantsMap = await cachedVariantsPromise;
                 const variants = variantsMap.get(String(collection.id));
                 if (variants) {
-                    const items = await this.productVariantService.applyPricesAndTranslateVariants(
-                        ctx,
-                        variants,
-                    );
-                    return {
-                        items,
-                        totalItems: items.length,
-                    };
+                    // Check if the requested relations are compatible with the cached data.
+                    // The cache was populated with default relations ['taxCategory'].
+                    // We can use the cache ONLY if the requested relations are a subset of or equal to the default relations.
+                    const defaultRelationsForCache = ['taxCategory'];
+                    const isCacheCompatible = relations.every(rel => defaultRelationsForCache.includes(rel));
+
+                    if (isCacheCompatible) {
+                        // Cache is compatible, use it.
+                        const { adminListQueryLimit } = this.configService.apiOptions;
+                        const skip = args.options?.skip ?? 0;
+                        const take = args.options?.take ?? adminListQueryLimit;
+                        const items = await this.productVariantService.applyPricesAndTranslateVariants(
+                            ctx,
+                            variants.slice(skip, skip + take),
+                        );
+                        return {
+                            items,
+                            totalItems: variants.length,
+                        };
+                    }
                 }
             }
         }
diff --git a/packages/core/src/config/auth/default-verification-token-strategy.ts b/packages/core/src/config/auth/default-verification-token-strategy.ts
@@ -1,4 +1,4 @@
-import ms, { type StringValue } from 'ms';
+import ms from 'ms';
 
 import { RequestContext } from '../../api/common/request-context';
 import { Injector } from '../../common';
@@ -43,7 +43,7 @@ export class DefaultVerificationTokenStrategy implements VerificationTokenStrate
         const { verificationTokenDuration } = this.configService.authOptions;
         const verificationTokenDurationInMs =
             typeof verificationTokenDuration === 'string'
-                ? ms(verificationTokenDuration as StringValue)
+                ? ms(verificationTokenDuration)
                 : verificationTokenDuration;
 
         const [generatedOn] = token.split('_');
diff --git a/packages/core/src/config/config.module.ts b/packages/core/src/config/config.module.ts
@@ -7,6 +7,7 @@ import { InjectableStrategy } from '../common/types/injectable-strategy';
 
 import { resetConfig } from './config-helpers';
 import { ConfigService } from './config.service';
+import { Logger } from './logger/vendure-logger';
 
 @Module({
     providers: [ConfigService],
@@ -15,7 +16,7 @@ import { ConfigService } from './config.service';
 export class ConfigModule implements OnApplicationBootstrap, OnApplicationShutdown {
     constructor(
         private configService: ConfigService,
-        @Optional() private moduleRef: ModuleRef,
+        @Optional() private moduleRef: ModuleRef | undefined,
     ) {}
 
     async onApplicationBootstrap() {
@@ -38,6 +39,9 @@ export class ConfigModule implements OnApplicationBootstrap, OnApplicationShutdo
 
     private async initInjectableStrategies() {
         if (!this.moduleRef) {
+            Logger.warn(
+                'ConfigModule: moduleRef missing — skipping initialization of injectable strategies.',
+            );
             return;
         }
         const injector = new Injector(this.moduleRef);
@@ -58,6 +62,9 @@ export class ConfigModule implements OnApplicationBootstrap, OnApplicationShutdo
 
     private async initConfigurableOperations() {
         if (!this.moduleRef) {
+            Logger.warn(
+                'ConfigModule: moduleRef missing — skipping initialization of configurable operations.',
+            );
             return;
         }
         const injector = new Injector(this.moduleRef);
diff --git a/packages/core/src/config/order/order-by-code-access-strategy.ts b/packages/core/src/config/order/order-by-code-access-strategy.ts
@@ -1,4 +1,4 @@
-import ms, { type StringValue } from 'ms';
+import ms from 'ms';
 
 import { RequestContext } from '../../api/common/request-context';
 import { InjectableStrategy } from '../../common/types/injectable-strategy';
@@ -68,7 +68,7 @@ export class DefaultOrderByCodeAccessStrategy implements OrderByCodeAccessStrate
         // For guest Customers, allow access to the Order for the following
         // time period
         const anonymousAccessPermitted = () => {
-            const anonymousAccessLimit = ms(this.anonymousAccessDuration as StringValue);
+            const anonymousAccessLimit = ms(this.anonymousAccessDuration);
             const orderPlaced = order.orderPlacedAt ? +order.orderPlacedAt : 0;
             const now = Date.now();
             return now - orderPlaced < anonymousAccessLimit;
diff --git a/packages/core/src/plugin/default-scheduler-plugin/default-scheduler-strategy.ts b/packages/core/src/plugin/default-scheduler-plugin/default-scheduler-strategy.ts
@@ -1,6 +1,6 @@
 import { UpdateScheduledTaskInput } from '@vendure/common/lib/generated-types';
 import { Cron } from 'croner';
-import ms, { type StringValue } from 'ms';
+import ms from 'ms';
 
 import { Injector } from '../../common';
 import { assertFound } from '../../common/utils';
@@ -85,7 +85,7 @@ export class DefaultSchedulerStrategy implements SchedulerStrategy {
             try {
                 this.runningTasks.push(task);
                 const timeout = task.options.timeout ?? (this.pluginOptions.defaultTimeout as number);
-                const timeoutMs = typeof timeout === 'number' ? timeout : ms(timeout as StringValue);
+                const timeoutMs = typeof timeout === 'number' ? timeout : ms(timeout);
 
                 let timeoutTimer: NodeJS.Timeout | undefined;
                 const timeoutPromise = new Promise((_, reject) => {
diff --git a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts
@@ -402,8 +402,8 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
                     qb.orWhere(existsClause.clause, existsClause.parameters);
                 }
             } else {
-                Logger.warn(
-                    `Could not build EXISTS subquery for custom property "${condition.isExistsCondition.customPropertyKey}". Skipping filter condition.`,
+                throw new Error(
+                    `Could not build EXISTS subquery for custom property "${condition.isExistsCondition.customPropertyKey}". This filter condition cannot be applied.`,
                 );
             }
             return;
diff --git a/packages/core/src/service/helpers/settings-store/settings-store.service.ts b/packages/core/src/service/helpers/settings-store/settings-store.service.ts
@@ -2,7 +2,7 @@ import { Injectable, OnModuleInit } from '@nestjs/common';
 import { ModuleRef } from '@nestjs/core';
 import { Permission, SettingsStoreScopeType } from '@vendure/common/lib/generated-types';
 import { JsonCompatible } from '@vendure/common/lib/shared-types';
-import ms, { type StringValue } from 'ms';
+import ms from 'ms';
 
 import { RequestContext } from '../../../api/common/request-context';
 import { InternalServerError, UserInputError } from '../../../common/error/errors';
@@ -463,7 +463,7 @@ export class SettingsStoreService implements OnModuleInit {
      * Parse a duration string (e.g., '7d', '30m', '2h') into a Date object.
      */
     private parseDuration(duration: string): Date {
-        const milliseconds = ms(duration as StringValue);
+        const milliseconds = ms(duration);
         if (!milliseconds) {
             throw new Error(`Invalid duration format: ${duration}. Use format like '7d', '2h', '30m'`);
         }
diff --git a/packages/core/src/service/services/collection.service.ts b/packages/core/src/service/services/collection.service.ts
@@ -490,7 +490,7 @@ export class CollectionService implements OnModuleInit {
         ctx: RequestContext,
         input: PreviewCollectionVariantsInput,
         options?: ListQueryOptions<ProductVariant>,
-        relations?: RelationPaths<Collection>,
+        relations?: RelationPaths<ProductVariant>,
     ): Promise<PaginatedList<ProductVariant>> {
         const applicableFilters = this.getCollectionFiltersFromInput(input);
         if (input.parentId && input.inheritFilters) {
@@ -503,13 +503,18 @@ export class CollectionService implements OnModuleInit {
             );
             applicableFilters.push(...parentFilters, ...ancestorFilters);
         }
-        let qb = this.listQueryBuilder.build(ProductVariant, options, {
-            relations: relations ?? ['taxCategory'],
-            channelId: ctx.channelId,
-            where: { deletedAt: IsNull() },
-            ctx,
-            entityAlias: 'productVariant',
-        });
+
+        let qb = this.listQueryBuilder.build(
+            ProductVariant,
+            options || { take: undefined, skip: undefined },
+            {
+                relations: relations ?? ['taxCategory'],
+                channelId: ctx.channelId,
+                where: { deletedAt: IsNull() },
+                ctx,
+                entityAlias: 'productVariant',
+            },
+        );
 
         const { collectionFilters } = this.configService.catalogOptions;
         for (const filterType of collectionFilters) {
@@ -1058,12 +1063,16 @@ export class CollectionService implements OnModuleInit {
             return new Map();
         }
 
-        const qb = this.listQueryBuilder.build(ProductVariant, options, {
-            relations: relations ?? ['taxCategory'],
-            channelId: ctx.channelId,
-            where: { deletedAt: IsNull() },
-            ctx,
-        });
+        const qb = this.listQueryBuilder.build(
+            ProductVariant,
+            { take: undefined, skip: undefined },
+            {
+                relations: relations ?? ['taxCategory'],
+                channelId: ctx.channelId,
+                where: { deletedAt: IsNull() },
+                ctx,
+            },
+        );
 
         // We explicitly join with the product to ensure we filter out soft-deleted products,
         // matching the behavior of other collection-related variant queries.
diff --git a/packages/core/src/service/services/session.service.ts b/packages/core/src/service/services/session.service.ts
@@ -1,7 +1,7 @@
 import { Injectable, OnApplicationBootstrap } from '@nestjs/common';
 import { ID } from '@vendure/common/lib/shared-types';
 import crypto from 'crypto';
-import ms, { type StringValue } from 'ms';
+import ms from 'ms';
 import { Brackets, EntitySubscriberInterface, InsertEvent, RemoveEvent, UpdateEvent } from 'typeorm';
 
 import { RequestContext } from '../../api/common/request-context';
@@ -50,7 +50,7 @@ export class SessionService implements EntitySubscriberInterface, OnApplicationB
 
         const { sessionDuration } = this.configService.authOptions;
         this.sessionDurationInMs =
-            typeof sessionDuration === 'string' ? ms(sessionDuration as StringValue) : sessionDuration;
+            typeof sessionDuration === 'string' ? ms(sessionDuration) : sessionDuration;
 
         // This allows us to register this class as a TypeORM Subscriber while also allowing
         // the injection on dependencies. See https://docs.nestjs.com/techniques/database#subscribers
@@ -186,7 +186,7 @@ export class SessionService implements EntitySubscriberInterface, OnApplicationB
     serializeSession(session: AuthenticatedSession | AnonymousSession): CachedSession {
         const { sessionCacheTTL } = this.configService.authOptions;
         const sessionCacheTTLSeconds =
-            typeof sessionCacheTTL === 'string' ? ms(sessionCacheTTL as StringValue) / 1000 : sessionCacheTTL;
+            typeof sessionCacheTTL === 'string' ? ms(sessionCacheTTL) / 1000 : sessionCacheTTL;
 
         const expiry = new Date().getTime() / 1000 + sessionCacheTTLSeconds;
         const serializedSession: CachedSession = {

Original file line number	Diff line number	Diff line change
`@@ -71,10 +71,11 @@ export class CollectionResolver {`
`71`	`71`	`const countsPromise = this.collectionService.getProductVariantCounts(ctx, collectionIds);`
`72`	`72`	`this.requestContextCache.set(ctx, CacheKey.CollectionVariantCounts, countsPromise);`
`73`	`73`	`}`
`74`		`- if (isFieldInSelection(info, 'variants')) {`
	`74`	`+ if (isFieldInSelection(info, 'productVariants')) {`
`75`	`75`	`const variantsPromise = this.collectionService.getProductVariantsForCollections(`
`76`	`76`	`ctx,`
`77`	`77`	`collectionIds,`
	`78`	`+ ['taxCategory'],`
`78`	`79`	`);`
`79`	`80`	`this.requestContextCache.set(ctx, CacheKey.CollectionVariants, variantsPromise);`
`80`	`81`	`}`
Original file line number	Diff line number	Diff line change
`@@ -402,8 +402,8 @@ export class ListQueryBuilder implements OnApplicationBootstrap {`
`402`	`402`	`qb.orWhere(existsClause.clause, existsClause.parameters);`
`403`	`403`	`}`
`404`	`404`	`} else {`
`405`		`- Logger.warn(`
`406`		- `Could not build EXISTS subquery for custom property "${condition.isExistsCondition.customPropertyKey}". Skipping filter condition.`,
	`405`	`+ throw new Error(`
	`406`	+ `Could not build EXISTS subquery for custom property "${condition.isExistsCondition.customPropertyKey}". This filter condition cannot be applied.`,
`407`	`407`	`);`
`408`	`408`	`}`
`409`	`409`	`return;`