Skip to content

Commit

Permalink
refactor(atomic): replace @stencil/store with in-house implementation (
Browse files Browse the repository at this point in the history
…#4814)

https://coveord.atlassian.net/browse/KIT-3815
## Why replace this dependency ?

We are replacing stencil with lit and @stencil/store has some
functionalities depending on stencil internal functions. It won't work
once we begin switching.

The implementation is here
#4814 (comment)

## Why all the changes for isAppLoaded ?

@stencil/store has a hidden functionality we were unknowingly depending
on. There is an [stencil
subscription](https://github.com/ionic-team/stencil-store/blob/main/src/subscriptions/stencil.ts)
that causes automatic rerenders. The subscription listens to every
component 'getting' a certain part of the state. It registers those
components and whenever that same part of the state is 'set', it
triggers a new render of those components. We were depending on it for
the `loadingFlags` part of the state.

In this PR I added `createAppLoadedListener` which takes in a callback
and calls it whenever loadingFlags are empty. In every component that
needs to listen to the loading flags, we add a `@State isAppLoaded` and
create the listener to that internal state. Whenever that internal state
is re triggered, it rerenders the component.

### Alternatives to `createAppLoadedListener`
1. We could create a decorator. That would mean implement the decorator
in both Stencil & Lit. We can always revisit this whenever we implement
Lit and figure out something better.
2. We could use Lit signals for this. It is exactly the signal use case.

---------

Co-authored-by: GitHub Actions Bot <>
  • Loading branch information
alexprudhomme authored Jan 16, 2025
1 parent 58a2824 commit cc9cd0f
Show file tree
Hide file tree
Showing 29 changed files with 206 additions and 73 deletions.
14 changes: 0 additions & 14 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -3107,5 +3107,4 @@ export declare interface AtomicTimeframeFacet extends Components.AtomicTimeframe



import type {} from '@coveo/atomic/components';
import type {} from '@coveo/atomic/components';
1 change: 0 additions & 1 deletion packages/atomic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@
"@coveo/headless": "3.13.2",
"@popperjs/core": "2.11.8",
"@salesforce-ux/design-system": "2.25.6",
"@stencil/store": "2.0.16",
"dayjs": "1.11.13",
"dompurify": "3.2.3",
"escape-html": "1.0.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ interface Data {
}

export type CommerceStore = BaseStore<Data> & {
isAppLoaded(): boolean;
unsetLoadingFlag(loadingFlag: string): void;
setLoadingFlag(flag: string): void;
isMobile(): boolean;
Expand All @@ -43,10 +42,6 @@ export function createCommerceStore(
return {
...store,

isAppLoaded() {
return !store.state.loadingFlags.length;
},

unsetLoadingFlag(loadingFlag: string) {
unsetLoadingFlag(store, loadingFlag);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
BindStateToController,
InitializeBindings,
} from '../../../utils/initialization-utils';
import {createAppLoadedListener} from '../../common/interface/store';
import {LoadMoreButton} from '../../common/load-more/button';
import {LoadMoreContainer} from '../../common/load-more/container';
import {LoadMoreGuard} from '../../common/load-more/guard';
Expand Down Expand Up @@ -49,6 +50,7 @@ export class AtomicLoadMoreProducts {
private productListingOrSearchState!: ProductListingState | SearchState;

@State() public error!: Error;
@State() private isAppLoaded = false;

public initialize() {
if (this.bindings.interfaceElement.type === 'product-listing') {
Expand All @@ -57,6 +59,10 @@ export class AtomicLoadMoreProducts {
this.listingOrSearch = buildSearch(this.bindings.engine);
}
this.pagination = this.listingOrSearch.pagination();

createAppLoadedListener(this.bindings.store, (isAppLoaded) => {
this.isAppLoaded = isAppLoaded;
});
}

private get lastProduct() {
Expand All @@ -73,7 +79,7 @@ export class AtomicLoadMoreProducts {
return (
<LoadMoreGuard
hasResults={this.paginationState.totalEntries > 0}
isLoaded={this.bindings.store.isAppLoaded()}
isLoaded={this.isAppLoaded}
>
<LoadMoreContainer>
<LoadMoreSummary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
InitializeBindings,
} from '../../../utils/initialization-utils';
import {randomID} from '../../../utils/utils';
import {createAppLoadedListener} from '../../common/interface/store';
import {
PagerNextButton,
PagerPageButton,
Expand Down Expand Up @@ -59,6 +60,7 @@ export class AtomicCommercePager
public pagerState!: PaginationState;

@State() error!: Error;
@State() private isAppLoaded = false;

@Event({
eventName: 'atomic/scrollToTop',
Expand Down Expand Up @@ -99,6 +101,9 @@ export class AtomicCommercePager
this.listingOrSearch = buildSearch(this.bindings.engine);
}
this.pager = this.listingOrSearch.pagination();
createAppLoadedListener(this.bindings.store, (isAppLoaded) => {
this.isAppLoaded = isAppLoaded;
});
}

private validateProps() {
Expand All @@ -120,7 +125,7 @@ export class AtomicCommercePager
<PagerGuard
hasError={false}
hasItems={this.pagerState.totalPages > 1}
isAppLoaded={this.bindings.store.isAppLoaded()}
isAppLoaded={this.isAppLoaded}
>
<PagerNavigation i18n={this.bindings.i18n}>
<PagerPreviousButton
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
} from '../../../utils/initialization-utils';
import {randomID} from '../../../utils/utils';
import {ResultsPlaceholdersGuard} from '../../common/atomic-result-placeholder/placeholders';
import {createAppLoadedListener} from '../../common/interface/store';
import {DisplayGrid} from '../../common/item-list/display-grid';
import {
DisplayTable,
Expand Down Expand Up @@ -89,6 +90,7 @@ export class AtomicCommerceProductList
@State() private resultTemplateRegistered = false;
@State() public error!: Error;
@State() private templateHasError = false;
@State() private isAppLoaded = false;

/**
* The desired number of placeholders to display while the product list is loading.
Expand Down Expand Up @@ -164,6 +166,9 @@ export class AtomicCommerceProductList
nextNewItemTarget: this.focusTarget,
store: this.bindings.store,
});
createAppLoadedListener(this.bindings.store, (isAppLoaded) => {
this.isAppLoaded = isAppLoaded;
});
}

@Listen('atomic/selectChildProduct')
Expand Down Expand Up @@ -201,7 +206,7 @@ export class AtomicCommerceProductList
density={this.density}
display={this.display}
imageSize={this.imageSize}
displayPlaceholders={!this.bindings.store.isAppLoaded()}
displayPlaceholders={!this.isAppLoaded}
numberOfPlaceholders={this.numberOfPlaceholders}
></ResultsPlaceholdersGuard>
{this.display === 'table'
Expand All @@ -215,7 +220,7 @@ export class AtomicCommerceProductList
}

private computeListDisplayClasses() {
const displayPlaceholders = !this.bindings.store.isAppLoaded();
const displayPlaceholders = !this.isAppLoaded;

return getItemListDisplayClasses(
this.display,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from '../../../utils/initialization-utils';
import {randomID} from '../../../utils/utils';
import {FieldsetGroup} from '../../common/fieldset-group';
import {createAppLoadedListener} from '../../common/interface/store';
import {Choices} from '../../common/items-per-page/choices';
import {
ChoiceIsNaNError,
Expand Down Expand Up @@ -59,6 +60,8 @@ export class AtomicCommerceProductsPerPage
private summaryState!: SearchSummaryState | ProductListingSummaryState;

@State() public error!: Error;
@State() private isAppLoaded = false;

private choices!: number[];
private readonly radioGroupName = randomID(
'atomic-commerce-products-per-page-'
Expand Down Expand Up @@ -103,6 +106,9 @@ export class AtomicCommerceProductsPerPage
this.pagination = controller.pagination(
this.initialChoice ? {options: {pageSize: this.initialChoice}} : {}
);
createAppLoadedListener(this.bindings.store, (isAppLoaded) => {
this.isAppLoaded = isAppLoaded;
});
}

private get label() {
Expand All @@ -114,7 +120,7 @@ export class AtomicCommerceProductsPerPage
<PagerGuard
hasError={this.summaryState.hasError}
hasItems={this.summaryState.hasProducts}
isAppLoaded={this.bindings.store.isAppLoaded()}
isAppLoaded={this.isAppLoaded}
>
<div class="flex items-center">
<Label>{this.label}</Label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ interface Data {
}

export type CommerceRecommendationStore = BaseStore<Data> & {
isAppLoaded(): boolean;
unsetLoadingFlag(loadingFlag: string): void;
setLoadingFlag(flag: string): void;
};
Expand All @@ -28,10 +27,6 @@ export function createCommerceRecommendationStore(): CommerceRecommendationStore
return {
...store,

isAppLoaded() {
return !store.state.loadingFlags.length;
},

unsetLoadingFlag(loadingFlag: string) {
unsetLoadingFlag(store, loadingFlag);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {ResultsPlaceholdersGuard} from '../../common/atomic-result-placeholder/p
import {Carousel} from '../../common/carousel';
import {Heading} from '../../common/heading';
import {Hidden} from '../../common/hidden';
import {createAppLoadedListener} from '../../common/interface/store';
import {DisplayGrid} from '../../common/item-list/display-grid';
import {DisplayWrapper} from '../../common/item-list/display-wrapper';
import {ItemDisplayGuard} from '../../common/item-list/item-display-guard';
Expand Down Expand Up @@ -82,6 +83,7 @@ export class AtomicCommerceRecommendationList
@State()
public recommendationsState!: RecommendationsState;
@State() public error!: Error;
@State() private isAppLoaded = false;
@State() private productTemplateRegistered = false;
@State() private templateHasError = false;
@State() private currentPage = 0;
Expand Down Expand Up @@ -199,6 +201,9 @@ export class AtomicCommerceRecommendationList
nextNewItemTarget: this.focusTarget,
store: this.bindings.store,
});
createAppLoadedListener(this.bindings.store, (isAppLoaded) => {
this.isAppLoaded = isAppLoaded;
});
}

@Listen('atomic/selectChildProduct')
Expand Down Expand Up @@ -331,7 +336,7 @@ export class AtomicCommerceRecommendationList
}

private computeListDisplayClasses() {
const displayPlaceholders = !this.bindings.store.isAppLoaded();
const displayPlaceholders = !this.isAppLoaded;

return getItemListDisplayClasses(
'grid',
Expand Down Expand Up @@ -392,7 +397,7 @@ export class AtomicCommerceRecommendationList
density={this.density}
display={this.display}
imageSize={this.imageSize}
displayPlaceholders={!this.bindings.store.isAppLoaded()}
displayPlaceholders={!this.isAppLoaded}
numberOfPlaceholders={
this.productsPerPage ?? this.recommendationsState.products.length
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
InitializeBindings,
} from '../../../../utils/initialization-utils';
import {FacetPlaceholder} from '../../../common/facets/facet-placeholder/facet-placeholder';
import {createAppLoadedListener} from '../../../common/interface/store';
import {CommerceBindings as Bindings} from '../../atomic-commerce-interface/atomic-commerce-interface';

/**
Expand Down Expand Up @@ -52,13 +53,17 @@ export class AtomicCommerceFacets implements InitializableComponent<Bindings> {
public facetGeneratorState!: FacetGeneratorState;

@State() public error!: Error;
@State() private isAppLoaded = false;

public initialize() {
this.validateProps();
const {engine} = this.bindings;
const controller = this.controllerBuilder(engine);
this.facetGenerator = controller.facetGenerator();
this.summary = controller.summary();
createAppLoadedListener(this.bindings.store, (isAppLoaded) => {
this.isAppLoaded = isAppLoaded;
});
}

private isProductListing() {
Expand Down Expand Up @@ -87,7 +92,7 @@ export class AtomicCommerceFacets implements InitializableComponent<Bindings> {
}

public render() {
if (!this.bindings.store.isAppLoaded()) {
if (!this.isAppLoaded) {
return [...Array.from({length: this.collapseFacetsAfter})].map(() => (
<FacetPlaceholder isCollapsed={false} numberOfValues={8} />
));
Expand Down
Loading

0 comments on commit cc9cd0f

Please sign in to comment.