Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(headless): refresh commerce recommendations server-side #4617

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

y-lakhdar
Copy link
Contributor

@y-lakhdar y-lakhdar commented Oct 31, 2024

Adding server-side rendering for commerce SSR package only.

  • Updated headless-react package to reflect the new changes
  • Updated headless-ssr-commerce sample to reflect the new changes

Features

Customizable Recommendation Controller

  • Users can specify which recommendation controller to refresh by providing the recommendation key (as defined in the engine configuration).
  • If no key is specified, the recommendation controller will not perform any queries.
image
  • Supports selecting recommendations using the build method.
image

Recommendation Name Validation

  • Validation against invalid recommendation names.
  • Prevention of duplicate recommendation names.

Code to increase readability

Factory methods

Moved factory methods into dedicated files to avoid bloating the commerce-engine-ssr file.
Examples include:

  • hydrated-state-factory.ts
  • static-state-factory.ts

Core Engine Types

Centralized all core engine types into types/core-engine.ts.

Recommendation State Factories

Created dedicated factories for handling static and hydrated states for recommendations (recommendation-static-state-factory.ts).

Improved Controller Handling

Simplified the logic for building controllers based on solution type to eliminate complex, hard-to-read conditional blocks (677fd45)

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

Copy link

github-actions bot commented Oct 31, 2024

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 241.4 241.4 0
commerce 346 346 0
search 413 413 0
insight 403.8 403.8 0
recommendation 253.6 253.6 0
ssr 406.5 406.5 0
ssr-commerce 358.3 361.1 0.8

SSR Progress

Use case SSR (#) CSR (#) Progress (%)
search 39 44 89
recommendation 0 4 0
case-assist 0 6 0
insight 0 27 0
commerce 0 15 0
Detailed logs search : buildInteractiveResult
search : buildInteractiveInstantResult
search : buildInteractiveRecentResult
search : buildInteractiveCitation
search : buildGeneratedAnswer
recommendation : missing SSR support
case-assist : missing SSR support
insight : missing SSR support
commerce : missing SSR support

@y-lakhdar y-lakhdar changed the title feat(headless ssr): refresh recommendations server-side feat(headless ssr)!: refresh recommendations server-side Nov 6, 2024
@y-lakhdar y-lakhdar changed the title feat(headless ssr)!: refresh recommendations server-side feat(headless ssr): refresh recommendations server-side Nov 6, 2024
@y-lakhdar y-lakhdar marked this pull request as ready for review November 18, 2024 09:45
@y-lakhdar y-lakhdar requested review from a team as code owners November 18, 2024 09:45
@alexprudhomme alexprudhomme self-assigned this Nov 18, 2024
@alexprudhomme alexprudhomme changed the title feat(headless ssr): refresh recommendations server-side feat(headless): server rendered commerce recommendations Nov 18, 2024
@alexprudhomme alexprudhomme changed the title feat(headless): server rendered commerce recommendations feat(headless): refresh recommendations server-side Nov 18, 2024
@alexprudhomme alexprudhomme changed the title feat(headless): refresh recommendations server-side feat(headless): refresh commerce recommendations server-side Nov 18, 2024
Copy link
Collaborator

@louis-bompart louis-bompart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nit pik.
It's quite complex tho for me to review this. @fbeaudoincoveo will probably understand it better than me.

Comment on lines +172 to +176
Object.entries(controllers)
.filter(([key, _]) => isRecommendationController(key))
.forEach(([_, controller]) =>
(controller as Recommendations).refresh?.()
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We go through (some) array items twice.
Replace

array.filter(filterFn).forEach(foreachCb)

by

for (const element of array) {
  if(!filterFn(element)) {
    continue;
  }
  foreachCb(element)
}

Comment on lines +23 to +51
> = TSolutionType extends SolutionType.recommendation
? {
/**
* Executes only the initial search for a given configuration, then returns a resumable snapshot of engine state along with the state of the controllers.
*
* Useful for static generation and server-side rendering.
*/
(
controllers: Array<keyof TControllers>
): Promise<EngineStaticState<TSearchAction, TControllersStaticState>>;

fromBuildResult: FromBuildResult<
TControllers,
FetchStaticStateOptions,
EngineStaticState<TSearchAction, TControllersStaticState>
>;
}
: {
/**
* Executes only the initial search for a given configuration, then returns a resumable snapshot of engine state along with the state of the controllers.
*
* Useful for static generation and server-side rendering.
*/
(
...params: OptionsTuple<
FetchStaticStateOptions &
EngineDefinitionControllersPropsOption<TControllersProps>
>
): Promise<EngineStaticState<TSearchAction, TControllersStaticState>>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the reason why we duplicated all of the factories to separate the types from ssr-engine. In the case of recommendation, during fetchStaticState, we are adding a controller key array prop for recommendations you want to fetch. There will never be a case where ...params will be useful for recommendations so we removed it. It was best to create a seperate interface that can take in solutionType and do the logic in it.

Comment on lines +80 to 84
const fetchRecommendationStaticState =
fetchRecommendationStaticStateFactory<TControllerDefinitions>(
controllerDefinitions,
getOptions()
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow the trail here through fetchRecommendationStaticStateFactory to understand the filtering of the recommendation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants