Skip to content

Commit

Permalink
perf: correctly memoize categories selectors
Browse files Browse the repository at this point in the history
  • Loading branch information
dhhyi committed Jun 21, 2020
1 parent 4c422aa commit 855a1e0
Show file tree
Hide file tree
Showing 25 changed files with 354 additions and 189 deletions.
16 changes: 15 additions & 1 deletion src/app/core/facades/shopping.facade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import { debounce, debounceTime, filter, map, switchMap, tap } from 'rxjs/operat
import { ProductListingID } from 'ish-core/models/product-listing/product-listing.model';
import { ProductCompletenessLevel, ProductHelper } from 'ish-core/models/product/product.model';
import { addProductToBasket } from 'ish-core/store/customer/basket';
import { getCategoryLoading, getSelectedCategory, getTopLevelCategories } from 'ish-core/store/shopping/categories';
import {
getCategory,
getCategoryLoading,
getNavigationCategories,
getSelectedCategory,
getTopLevelCategories,
} from 'ish-core/store/shopping/categories';
import {
addToCompare,
getCompareProducts,
Expand Down Expand Up @@ -54,6 +60,14 @@ export class ShoppingFacade {
selectedCategory$ = this.store.pipe(select(getSelectedCategory));
selectedCategoryLoading$ = this.store.pipe(select(getCategoryLoading), debounceTime(500));

category$(uniqueId: string) {
return this.store.pipe(select(getCategory(uniqueId)));
}

navigationCategories$(uniqueId: string) {
return this.store.pipe(select(getNavigationCategories(uniqueId)));
}

// PRODUCT

selectedProduct$ = this.store.pipe(select(getSelectedProduct));
Expand Down
32 changes: 7 additions & 25 deletions src/app/core/models/category-view/category-view.model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ describe('Category View Model', () => {
]);

const view = createCategoryView(tree, '123');
expect(view.hasChildren()).toBeFalse();
expect(view.children()).toBeEmpty();
expect(view.hasChildren).toBeFalse();
expect(view.children).toBeEmpty();
});

const cat1 = {
Expand All @@ -60,34 +60,16 @@ describe('Category View Model', () => {
const tree = categoryTree([cat1, cat11]);

const view = createCategoryView(tree, '123');
expect(view.hasChildren()).toBeTrue();
expect(view.children()).toHaveLength(1);
expect(view.hasChildren).toBeTrue();
expect(view.children).toHaveLength(1);

expect(view.children()[0].uniqueId).toEqual('123.456');
expect(view.children[0]).toEqual('123.456');
});

it('should provide methods to check if a node in a deep complex tree has children', () => {
const tree = categoryTree([cat1, cat11, cat111]);

const view = createCategoryView(tree, '123');
expect(view.hasChildren()).toBeTrue();
expect(view.children()).toHaveLength(1);

const subCategory = view.children()[0];
expect(subCategory.uniqueId).toEqual('123.456');
expect(subCategory.hasChildren()).toBeTrue();
expect(subCategory.children()).toHaveLength(1);

const subSubCategory = subCategory.children()[0];
expect(subSubCategory.uniqueId).toEqual('123.456.789');
expect(subSubCategory.hasChildren()).toBeFalse();
expect(subSubCategory.children()).toBeEmpty();
});

it('should provide acces to the category path of a category', () => {
it('should provide access to the category path of a category', () => {
const tree = categoryTree([cat1, cat11, cat111]);
const view = createCategoryView(tree, '123.456.789');

expect(view.pathCategories().map(v => v.uniqueId)).toEqual(['123', '123.456', '123.456.789']);
expect(view.categoryPath).toEqual(['123', '123.456', '123.456.789']);
});
});
10 changes: 4 additions & 6 deletions src/app/core/models/category-view/category-view.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import { Category } from 'ish-core/models/category/category.model';
* View on a {@link Category} with additional methods for navigating to sub categories or category path
*/
export interface CategoryView extends Category {
children(): CategoryView[];
hasChildren(): boolean;
pathCategories(): CategoryView[];
children: string[];
hasChildren: boolean;
}

export function createCategoryView(tree: CategoryTree, uniqueId: string): CategoryView {
Expand All @@ -20,8 +19,7 @@ export function createCategoryView(tree: CategoryTree, uniqueId: string): Catego

return {
...tree.nodes[uniqueId],
hasChildren: () => !!tree.edges[uniqueId] && !!tree.edges[uniqueId].length,
children: () => (tree.edges[uniqueId] || []).map(id => createCategoryView(tree, id)),
pathCategories: () => tree.nodes[uniqueId].categoryPath.map(id => createCategoryView(tree, id)),
children: tree.edges[uniqueId] || [],
hasChildren: !!tree.edges[uniqueId] && !!tree.edges[uniqueId].length,
};
}
8 changes: 4 additions & 4 deletions src/app/core/routing/category/category.route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import { UrlMatchResult, UrlSegment } from '@angular/router';
import { MonoTypeOperatorFunction } from 'rxjs';
import { filter } from 'rxjs/operators';

import { CategoryView } from 'ish-core/models/category-view/category-view.model';
import { Category } from 'ish-core/models/category/category.model';
import { CoreState } from 'ish-core/store/core/core-store';
import { selectRouteParam } from 'ish-core/store/core/router';

export function generateLocalizedCategorySlug(category: CategoryView) {
export function generateLocalizedCategorySlug(category: Category) {
if (!category || !category.categoryPath.length) {
return '';
}
const lastCat = category.pathCategories()[category.categoryPath.length - 1].name;
const lastCat = category.name;
return lastCat ? lastCat.replace(/ /g, '-') : '';
}

Expand All @@ -37,7 +37,7 @@ export function matchCategoryRoute(segments: UrlSegment[]): UrlMatchResult {
return;
}

export function generateCategoryUrl(category: CategoryView): string {
export function generateCategoryUrl(category: Category): string {
if (!category) {
return '/';
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ import {
loadTopLevelCategoriesSuccess,
} from './categories.actions';
import {
getCategory,
getCategoryEntities,
getCategoryLoading,
getNavigationCategories,
getSelectedCategory,
getTopLevelCategories,
isTopLevelCategoriesLoaded,
Expand All @@ -35,8 +37,7 @@ describe('Categories Selectors', () => {

beforeEach(() => {
prod = { sku: 'sku' } as Product;
cat = { uniqueId: 'Aa', categoryPath: ['Aa'] } as Category;
cat.hasOnlineProducts = true;
cat = { uniqueId: 'Aa', categoryPath: ['Aa'], hasOnlineProducts: true } as Category;

@Component({ template: 'dummy' })
class DummyComponent {}
Expand All @@ -63,6 +64,7 @@ describe('Categories Selectors', () => {

it('should not select any selected category when used', () => {
expect(getSelectedCategory(store$.state)).toBeUndefined();
expect(getCategory(cat.uniqueId)(store$.state)).toBeUndefined();
});

it('should not select any top level categories when used', () => {
Expand Down Expand Up @@ -117,6 +119,7 @@ describe('Categories Selectors', () => {

it('should not select the irrelevant category when used', () => {
expect(getSelectedCategory(store$.state)).toBeUndefined();
expect(getCategory(cat.uniqueId)(store$.state).uniqueId).toEqual(cat.uniqueId);
});
});

Expand All @@ -133,18 +136,22 @@ describe('Categories Selectors', () => {

it('should select the selected category when used', () => {
expect(getSelectedCategory(store$.state).uniqueId).toEqual(cat.uniqueId);
expect(getCategory(cat.uniqueId)(store$.state).uniqueId).toEqual(cat.uniqueId);
});
});
});

describe('loading top level categories', () => {
let catA: Category;
let catB: Category;

beforeEach(() => {
catA = { uniqueId: 'A', categoryPath: ['A'] } as Category;
catB = { uniqueId: 'B', categoryPath: ['B'] } as Category;
store$.dispatch(loadTopLevelCategoriesSuccess({ categories: categoryTree([catA, catB]) }));
const catA = { name: 'name_A', uniqueId: 'A', categoryPath: ['A'] } as Category;
const catA1 = { name: 'name_A.1', uniqueId: 'A.1', categoryPath: ['A', 'A.1'] } as Category;
const catA1a = { name: 'name_A.1.a', uniqueId: 'A.1.a', categoryPath: ['A', 'A.1', 'A.1.a'] } as Category;
const catA1b = { name: 'name_A.1.b', uniqueId: 'A.1.b', categoryPath: ['A', 'A.1', 'A.1.b'] } as Category;
const catA2 = { name: 'name_A.2', uniqueId: 'A.2', categoryPath: ['A', 'A.2'] } as Category;
const catB = { name: 'name_B', uniqueId: 'B', categoryPath: ['B'] } as Category;
store$.dispatch(
loadTopLevelCategoriesSuccess({ categories: categoryTree([catA, catA1, catA1a, catA1b, catA2, catB]) })
);
});

it('should select root categories when used', () => {
Expand All @@ -154,5 +161,68 @@ describe('Categories Selectors', () => {
it('should remember if top level categories are loaded', () => {
expect(isTopLevelCategoriesLoaded(store$.state)).toBeTrue();
});

describe('selecting navigation categories', () => {
it('should select top level categories when no argument was supplied', () => {
expect(getNavigationCategories(undefined)(store$.state)).toMatchInlineSnapshot(`
Array [
Object {
"hasChildren": true,
"name": "name_A",
"uniqueId": "A",
"url": "/name_A-catA",
},
Object {
"hasChildren": false,
"name": "name_B",
"uniqueId": "B",
"url": "/name_B-catB",
},
]
`);
});

it('should select sub categories when sub category is selected', () => {
expect(getNavigationCategories('A')(store$.state)).toMatchInlineSnapshot(`
Array [
Object {
"hasChildren": true,
"name": "name_A.1",
"uniqueId": "A.1",
"url": "/name_A.1-catA.1",
},
Object {
"hasChildren": false,
"name": "name_A.2",
"uniqueId": "A.2",
"url": "/name_A.2-catA.2",
},
]
`);
});

it('should select deeper sub categories when deeper sub category is selected', () => {
expect(getNavigationCategories('A.1')(store$.state)).toMatchInlineSnapshot(`
Array [
Object {
"hasChildren": false,
"name": "name_A.1.a",
"uniqueId": "A.1.a",
"url": "/name_A.1.a-catA.1.a",
},
Object {
"hasChildren": false,
"name": "name_A.1.b",
"uniqueId": "A.1.b",
"url": "/name_A.1.b-catA.1.b",
},
]
`);
});

it('should be empty when selecting leaves', () => {
expect(getNavigationCategories('A.1.a')(store$.state)).toBeEmpty();
});
});
});
});
46 changes: 44 additions & 2 deletions src/app/core/store/shopping/categories/categories.selectors.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { createSelector } from '@ngrx/store';
import { createSelector, createSelectorFactory, defaultMemoize } from '@ngrx/store';
import { isEqual } from 'lodash-es';

import { CategoryTree, CategoryTreeHelper } from 'ish-core/models/category-tree/category-tree.model';
import { createCategoryView } from 'ish-core/models/category-view/category-view.model';
import { generateCategoryUrl } from 'ish-core/routing/category/category.route';
import { selectRouteParam } from 'ish-core/store/core/router';
import { ShoppingState, getShoppingState } from 'ish-core/store/shopping/shopping-store';

Expand All @@ -13,10 +16,21 @@ export const getCategoryTree = createSelector(getCategoriesState, state => state
*/
export const getCategoryEntities = createSelector(getCategoryTree, tree => tree.nodes);

const getCategorySubTree = (uniqueId: string) =>
createSelectorFactory(projector =>
defaultMemoize(projector, CategoryTreeHelper.equals, CategoryTreeHelper.equals)
)(getCategoryTree, (tree: CategoryTree) => CategoryTreeHelper.subTree(tree, uniqueId));

export const getCategory = (uniqueId: string) =>
createSelectorFactory(projector => defaultMemoize(projector, CategoryTreeHelper.equals, isEqual))(
getCategorySubTree(uniqueId),
(tree: CategoryTree) => createCategoryView(tree, uniqueId)
);

/**
* Retrieves the currently resolved selected category.
*/
export const getSelectedCategory = createSelector(
export const getSelectedCategory = createSelectorFactory(projector => defaultMemoize(projector, undefined, isEqual))(
getCategoryTree,
selectRouteParam('categoryUniqueId'),
createCategoryView
Expand All @@ -29,3 +43,31 @@ export const getTopLevelCategories = createSelector(getCategoryTree, tree =>
);

export const isTopLevelCategoriesLoaded = createSelector(getCategoriesState, state => state.topLevelLoaded);

export interface NavigationCategory {
uniqueId: string;
name: string;
url: string;
hasChildren: boolean;
}

function mapNavigationCategoryFromId(uniqueId: string): NavigationCategory {
return {
uniqueId,
name: this.nodes[uniqueId].name,
url: generateCategoryUrl(this.nodes[uniqueId]),
hasChildren: !!this.edges[uniqueId]?.length,
};
}

export const getNavigationCategories = (uniqueId: string) =>
createSelectorFactory(projector => defaultMemoize(projector, CategoryTreeHelper.equals, isEqual))(
getCategoryTree,
(tree: CategoryTree): NavigationCategory[] => {
if (!uniqueId) {
return tree.rootIds.map(mapNavigationCategoryFromId.bind(tree));
}
const subTree = CategoryTreeHelper.subTree(tree, uniqueId);
return subTree.edges[uniqueId] ? subTree.edges[uniqueId].map(mapNavigationCategoryFromId.bind(subTree)) : [];
}
);
8 changes: 0 additions & 8 deletions src/app/core/store/shopping/shopping-store.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,6 @@ describe('Shopping Store', () => {
categories: tree(A,A.123)
[Shopping] Load Category Success:
categories: tree(A.123,A.123.456)
[Shopping] Load Category:
categoryId: "A.123"
[Shopping] Load Category Success:
categories: tree(A.123,A.123.456)
@ngrx/router-store/navigated:
routerState: {"url":"/category/A.123.456","params":{"categoryUniqueId":"A...
event: {"id":1,"url":"/category/A.123.456"}
Expand Down Expand Up @@ -720,10 +716,6 @@ describe('Shopping Store', () => {
categories: tree(A,A.123)
[Shopping] Load Category Success:
categories: tree(A.123,A.123.456)
[Shopping] Load Category:
categoryId: "A.123"
[Shopping] Load Category Success:
categories: tree(A.123,A.123.456)
@ngrx/router-store/navigated:
routerState: {"url":"/category/A.123.456/product/P1","params":{"categoryU...
event: {"id":1,"url":"/category/A.123.456/product/P1"}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div class="row">
<div class="col-md-3">
<div class="navigation-panel" [ngbCollapse]="isCollapsed">
<ish-category-navigation [category]="category" [categoryNavigationLevel]="0"></ish-category-navigation>
<ish-category-navigation></ish-category-navigation>
</div>
<a class="d-md-none mobile-filter-toggle" (click)="toggle()">
{{ 'search.mobile.filter.trigger' | translate }}
Expand All @@ -13,7 +13,7 @@
<div class="col-md-9">
<ish-breadcrumb [category]="category"></ish-breadcrumb>
<h1>{{ category.name }}</h1>
<ish-category-list [categories]="category.children()"></ish-category-list>
<ish-category-list [categories]="category.children"></ish-category-list>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<ul class="category-list row">
<li *ngFor="let category of categories; trackBy: trackByFn" class="category-list-item col-6 col-lg-4">
<ish-category-tile [category]="category"></ish-category-tile>
<li *ngFor="let category of categories" class="category-list-item col-6 col-lg-4">
<ish-category-tile [categoryUniqueId]="category"></ish-category-tile>
</li>
</ul>
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
import { ChangeDetectionStrategy, Component, Input } from '@angular/core';

import { CategoryView } from 'ish-core/models/category-view/category-view.model';
import { Category } from 'ish-core/models/category/category.model';

@Component({
selector: 'ish-category-list',
templateUrl: './category-list.component.html',
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class CategoryListComponent {
@Input() categories: Category[];

trackByFn(_, item: CategoryView) {
return item.uniqueId;
}
@Input() categories: string[];
}
Loading

0 comments on commit 855a1e0

Please sign in to comment.