Skip to content

Commit

Permalink
fix(headless SSR): validate controller props (#4796)
Browse files Browse the repository at this point in the history
Return a clear error message if the controller props is undefined


![image](https://github.com/user-attachments/assets/3856036d-b3b7-4355-8e47-2c51d8c1129b)

https://coveord.atlassian.net/browse/KIT-3809

---------

Co-authored-by: Frederic Beaudoin <[email protected]>
Co-authored-by: Louis Bompart <[email protected]>
  • Loading branch information
3 people authored Jan 15, 2025
1 parent eec0326 commit 3e9f466
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export interface ControllerDefinitionWithProps<
*/
buildWithProps(
engine: SSRCommerceEngine,
props: TProps,
props?: TProps,
solutionType?: SolutionType
): TController & ControllerWithKind;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/headless/src/app/ssr-engine/types/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export interface ControllerDefinitionWithProps<
* @param props - The controller properties.
* @returns The controller.
*/
buildWithProps(engine: TEngine, props: TProps): TController;
buildWithProps(engine: TEngine, props?: TProps): TController;
}

export type ControllerDefinition<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
createControllerWithKind,
Kind,
} from '../../../../app/commerce-ssr-engine/types/kind.js';
import {MissingControllerProps} from '../../../../utils/errors.js';
import {Cart, buildCart, CartInitialState} from './headless-cart.js';

export type {CartState, CartItem, CartProps} from './headless-cart.js';
Expand Down Expand Up @@ -30,6 +31,9 @@ export function defineCart(): CartDefinition {
standalone: true,
recommendation: true,
buildWithProps: (engine, props) => {
if (props === undefined) {
throw new MissingControllerProps(Kind.Cart);
}
const controller = buildCart(engine, {initialState: props.initialState});
return createControllerWithKind(controller, Kind.Cart);
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import {SSRCommerceEngine} from '../../../app/commerce-ssr-engine/factories/build-factory.js';
import {buildMockCommerceState} from '../../../test/mock-commerce-state.js';
import {buildMockSSRCommerceEngine} from '../../../test/mock-engine-v2.js';
import {MissingControllerProps} from '../../../utils/errors.js';
import {buildContext, ContextOptions, Context} from './headless-context.js';
import {ContextDefinition, defineContext} from './headless-context.ssr.js';

vi.mock('./headless-context');
const buildContextMock = vi.mocked(buildContext);

describe('define commerce context', () => {
const options: ContextOptions = {
language: 'en',
country: 'us',
currency: 'USD',
view: {
url: 'https://example.org',
},
};
let contextDefinition: ContextDefinition;

beforeEach(() => {
buildContextMock.mockReturnValue({} as Context);
contextDefinition = defineContext();
});

afterEach(() => {
buildContextMock.mockClear();
});

it('defineContext returns the proper type', () => {
expect(contextDefinition).toMatchObject<ContextDefinition>({
buildWithProps: expect.any(Function),
listing: true,
search: true,
standalone: true,
recommendation: true,
});
});

it('buildWithProps should pass its parameters to the buildContext', () => {
const engine: SSRCommerceEngine = buildMockSSRCommerceEngine({
...buildMockCommerceState(),
commerceContext: {...options},
});

contextDefinition.buildWithProps(engine, options);

expect(buildContextMock).toBeCalledWith(engine, {options});
});

it('should throw when props is undefined', () => {
const engine: SSRCommerceEngine = buildMockSSRCommerceEngine({
...buildMockCommerceState(),
commerceContext: {...options},
});
const props = undefined as unknown as ContextOptions;

expect(() => {
contextDefinition.buildWithProps(engine, props);
}).toThrow(MissingControllerProps);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
createControllerWithKind,
Kind,
} from '../../../app/commerce-ssr-engine/types/kind.js';
import {MissingControllerProps} from '../../../utils/errors.js';
import {
Context,
buildContext,
Expand Down Expand Up @@ -32,6 +33,9 @@ export function defineContext(): ContextDefinition {
standalone: true,
recommendation: true,
buildWithProps: (engine, props) => {
if (props === undefined) {
throw new MissingControllerProps(Kind.Context);
}
const controller = buildContext(engine, {options: props});
return createControllerWithKind(controller, Kind.Context);
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,32 @@
import {SSRSearchEngine} from '../../app/search-engine/search-engine.ssr.js';
import {ControllerDefinitionWithProps} from '../../app/ssr-engine/types/common.js';
import {buildMockSSRSearchEngine} from '../../test/mock-engine-v2.js';
import {createMockState} from '../../test/mock-state.js';
import {Context, buildContext} from './headless-context.js';
import {ContextProps, defineContext} from './headless-context.ssr.js';
import {MissingControllerProps} from '../../utils/errors.js';
import {buildContext} from './headless-context.js';
import {
ContextDefinition,
ContextProps,
defineContext,
} from './headless-context.ssr.js';

vi.mock('./headless-context');
const buildContextMock = vi.mocked(buildContext);

type contextDefinitionType = ControllerDefinitionWithProps<
SSRSearchEngine,
Context,
ContextProps
>;

describe('define context', () => {
let contextDefinition: contextDefinitionType;
let contextDefinition: ContextDefinition;

beforeEach(() => {
contextDefinition = defineContext();
buildContextMock.mockClear();
});

it('defineContext returns the proper type', () => {
expect(contextDefinition).toMatchObject<contextDefinitionType>({
expect(contextDefinition).toMatchObject<ContextDefinition>({
buildWithProps: expect.any(Function),
});
});

it("buildWithProps should pass it's parameters to the buildContext", () => {
it('buildWithProps should pass its parameters to the buildContext', () => {
const engine: SSRSearchEngine = buildMockSSRSearchEngine(createMockState());
const props: ContextProps = {} as unknown as ContextProps;

Expand All @@ -38,4 +36,13 @@ describe('define context', () => {

expect(buildContextMock).toBeCalledWith(engine, props);
});

it('should throw when props is undefined', () => {
const engine: SSRSearchEngine = buildMockSSRSearchEngine(createMockState());
const props: ContextProps = undefined as unknown as ContextProps;

expect(() => {
contextDefinition.buildWithProps(engine, props);
}).toThrow(MissingControllerProps);
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {SearchEngine} from '../../app/search-engine/search-engine.js';
import {ControllerDefinitionWithProps} from '../../app/ssr-engine/types/common.js';
import {MissingControllerProps} from '../../utils/errors.js';
import {ContextProps} from '../core/context/headless-core-context.js';
import {Context, buildContext} from './headless-context.js';

Expand All @@ -16,7 +17,11 @@ export interface ContextDefinition
* */
export function defineContext(): ContextDefinition {
return {
buildWithProps: (engine, props) =>
buildContext(engine, {initialState: props.initialState}),
buildWithProps: (engine, props) => {
if (props === undefined) {
throw new MissingControllerProps('Context');
}
return buildContext(engine, {initialState: props.initialState});
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {queryReducer as query} from '../../features/query/query-slice.js';
import {sortCriteriaReducer as sortCriteria} from '../../features/sort-criteria/sort-criteria-slice.js';
import {staticFilterSetReducer as staticFilterSet} from '../../features/static-filter-set/static-filter-set-slice.js';
import {tabSetReducer as tabSet} from '../../features/tab-set/tab-set-slice.js';
import {loadReducerError} from '../../utils/errors.js';
import {loadReducerError, MissingControllerProps} from '../../utils/errors.js';
import {advancedSearchQueriesReducer as advancedSearchQueries} from './../../features/advanced-search-queries/advanced-search-queries-slice.js';
import {
SearchParameterManager,
Expand Down Expand Up @@ -44,6 +44,9 @@ export interface SearchParameterManagerDefinition
export function defineSearchParameterManager(): SearchParameterManagerDefinition {
return {
buildWithProps: (engine, props) => {
if (props === undefined) {
throw new MissingControllerProps('SearchParameterManager');
}
if (!loadSearchParameterManagerReducers(engine)) {
throw loadReducerError;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {SearchEngine} from '../../app/search-engine/search-engine.js';
import {ControllerDefinitionWithProps} from '../../app/ssr-engine/types/common.js';
import {MissingControllerProps} from '../../utils/errors.js';
import {
UrlManager,
UrlManagerInitialState,
Expand All @@ -25,6 +26,10 @@ export const defineUrlManager = (): ControllerDefinitionWithProps<
UrlManager,
UrlManagerBuildProps
> => ({
buildWithProps: (engine, props) =>
buildUrlManager(engine, {initialState: props.initialState}),
buildWithProps: (engine, props) => {
if (props === undefined) {
throw new MissingControllerProps('UrlManager');
}
return buildUrlManager(engine, {initialState: props.initialState});
},
});
9 changes: 9 additions & 0 deletions packages/headless/src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ export class InvalidControllerDefinition extends Error {
}
}

export class MissingControllerProps extends Error {
constructor(controller: string) {
super();
this.name = 'MissingControllerProps';
this.message = `${controller} props are required but were undefined. Ensure they are included when calling \`fetchStaticState\` or \`hydrateStaticState\`.`;
// + '\nSee [TODO: add link to fetchStaticState example] for more information.';
}
}

export class MultipleRecommendationError extends Error {
constructor(slotId: string) {
super();
Expand Down

0 comments on commit 3e9f466

Please sign in to comment.