fix(react|redux): commerce pages in ssr not working by richard190m · Pull Request #960 · Farfetch/blackout

richard190m · 2024-01-09T14:52:31Z

Description

Fix commerce pages on server side by:

Fix serverInitialState to include strategy prop
Fix hooks useCommercePages and useContentPages to use slug as hash to redux store

Dependencies

Checklist

The commit message follows our guidelines
Tests for the respective changes have been added
The code is commented, particularly in hard-to-understand areas
The labels and/or milestones were added

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

boliveira · 2024-01-15T14:40:29Z

packages/redux/src/contents/actions/factories/fetchCommercePagesFactory.ts

      hash = generateContentHash({
        contentTypeCode: ContentTypeCode.CommercePages,
-        ...query,
+        codes: slug,


This will make requests for commerce pages containing different query parameters to be indexed on the same entry in the redux store. Is this ok?

This only generate an hash differently. The hash for commerce pages need to be the url instead of query params

boliveira · 2024-01-15T14:41:11Z

packages/redux/src/contents/actions/factories/fetchCommercePagesFactory.ts

  (getCommercePages: GetCommercePages) =>
  (
-    query: QueryCommercePages,
+    query: QueryCommercePagesWithSlug,


This will be a breaking change, can't you calculate a default value for the slug or at least make it optional?

Only BG is using the commerce pages of this package version. We can change this an talk to them to implement this

Our boilerplate is also making use of this, so we would have to update it as well.

boliveira · 2024-01-15T14:48:22Z

packages/react/src/contents/hooks/useContentPage.ts

    () => ({
-      contentTypeCode: ContentTypeCode.ContentPage,
-      codes: fetchQuery.slug.split('?')[0] as string,
+      contentTypeCode: isCommercePage


I do not understand this approach. Why would the user need to set isCommercePage to true if there is a useCommercePages hook for that?

Currently, to distinguish from the fetch to commerce pages endpoint directly we use the contentType commercePages and for the request to Serverless API, we use contentType contentPages, to generate the hash

Ok, but in that case, why not use the useCommercePages hook by itself? I think that this signature is leaking implementation details that the consumer should not have to worry about. In my opinion, the correct solution would be for the serverInitialState to generate entries for both contentTypes in the store so the consumer does not need to worry about this.
What I mean is, the serverInitialState would set an entry for the commercePages and contentPages contentTypes containing the same data. I think they have the same data format so it would work. If that assumption is not correct, then we should adapt the responses from FO to the proper format.

boliveira · 2024-01-15T14:49:22Z

packages/react/src/contents/hooks/types/useCommercePages.types.ts

 import type { Config, QueryCommercePages } from '@farfetch/blackout-client';

 export interface UseCommercePagesOptions extends QueryCommercePages {
+  slug: string;


This will be a breaking change as well, can't it be at least defined as optional?

As mentioned before, this is only used by BG and this will fix the commerce pages in this package version

packages/redux/src/contents/serverInitialState.ts

boliveira · 2024-01-15T15:06:46Z

packages/redux/src/contents/serverInitialState.ts


  const { searchContentRequests, slug, seoMetadata, subfolder } = model;
+  const url = subfolder !== '/' ? slug?.replace(subfolder, '') : slug;
+  const normalizedUrl = url


Is this necessary? If json=true is passed, the response returned by FO will not be html, so the application will not be run in this case.

richard190m added 📦 react Relative to the `@farfetch/blackout-react` package 📦 redux Relative to the `@farfetch/blackout-redux` package labels Jan 9, 2024

github-actions bot added the type: bug Something isn't working properly label Jan 9, 2024

richard190m force-pushed the fix/serverInitialState-commerce-pages branch from 464f0ce to ca1a08e Compare January 9, 2024 14:52

github-actions bot added type: bug Something isn't working properly and removed type: bug Something isn't working properly labels Jan 9, 2024

richard190m changed the title ~~fix: commerce pages in ssr~~ fix(react|redux): commerce pages in ssr Jan 9, 2024

richard190m force-pushed the fix/serverInitialState-commerce-pages branch from ca1a08e to 47eab25 Compare January 10, 2024 15:25

github-actions bot added type: bug Something isn't working properly and removed type: bug Something isn't working properly labels Jan 10, 2024

richard190m force-pushed the fix/serverInitialState-commerce-pages branch from 47eab25 to 14a1d68 Compare January 10, 2024 15:27

github-actions bot added type: bug Something isn't working properly and removed type: bug Something isn't working properly labels Jan 10, 2024

richard190m requested review from edupaulos, joaoprcosta and vinicaval January 10, 2024 15:27

richard190m marked this pull request as ready for review January 10, 2024 15:28

richard190m changed the title ~~fix(react|redux): commerce pages in ssr~~ fix(react|redux): commerce pages in ssr not working Jan 10, 2024

joaoprcosta approved these changes Jan 10, 2024

View reviewed changes

vinicaval approved these changes Jan 10, 2024

View reviewed changes

edupaulos approved these changes Jan 10, 2024

View reviewed changes

richard190m force-pushed the fix/serverInitialState-commerce-pages branch from 14a1d68 to 2429d79 Compare January 10, 2024 15:35

github-actions bot added type: bug Something isn't working properly and removed type: bug Something isn't working properly labels Jan 10, 2024

richard190m force-pushed the fix/serverInitialState-commerce-pages branch from 2429d79 to 8f7947e Compare January 10, 2024 16:02

github-actions bot added type: bug Something isn't working properly and removed type: bug Something isn't working properly labels Jan 10, 2024

fix(react|redux): commerce pages in ssr not working

d0b2abc

richard190m force-pushed the fix/serverInitialState-commerce-pages branch from 8f7947e to d0b2abc Compare January 10, 2024 16:22

github-actions bot removed the type: bug Something isn't working properly label Jan 10, 2024

github-actions bot added the type: bug Something isn't working properly label Jan 10, 2024

boliveira reviewed Jan 15, 2024

View reviewed changes

packages/redux/src/contents/serverInitialState.ts Show resolved Hide resolved

boliveira reviewed Jan 15, 2024

View reviewed changes

dnamorim added this to the dev-main-23-01-24 milestone Jan 26, 2024

dnamorim changed the base branch from main to dev-main-23-01-24 January 26, 2024 14:27

dnamorim force-pushed the dev-main-23-01-24 branch 3 times, most recently from 1729a08 to b9d6190 Compare January 29, 2024 11:29

Base automatically changed from dev-main-23-01-24 to main January 30, 2024 17:39

boliveira changed the base branch from main to dev-main-06-02-2024 January 30, 2024 17:42

boliveira modified the milestones: dev-main-23-01-24, dev-main-06-02-2024 Jan 30, 2024

Base automatically changed from dev-main-06-02-2024 to main February 6, 2024 16:59

dnamorim changed the base branch from main to dev-main-13-02-24 February 6, 2024 17:10

dnamorim modified the milestones: dev-main-06-02-2024, dev-main-13-02-24 Feb 6, 2024

Base automatically changed from dev-main-13-02-24 to main February 14, 2024 17:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(react|redux): commerce pages in ssr not working#960

fix(react|redux): commerce pages in ssr not working#960
richard190m wants to merge 1 commit intomainfrom
fix/serverInitialState-commerce-pages

richard190m commented Jan 9, 2024

Uh oh!

boliveira Jan 15, 2024

Uh oh!

richard190m Jan 19, 2024

Uh oh!

boliveira Jan 15, 2024

Uh oh!

richard190m Jan 19, 2024

Uh oh!

boliveira Jan 19, 2024

Uh oh!

boliveira Jan 15, 2024

Uh oh!

richard190m Jan 19, 2024

Uh oh!

boliveira Jan 19, 2024 •

edited

Loading

Uh oh!

boliveira Jan 15, 2024

Uh oh!

richard190m Jan 19, 2024

Uh oh!

Uh oh!

boliveira Jan 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

Conversation

richard190m commented Jan 9, 2024

Description

Dependencies

Checklist

Disclaimer

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boliveira Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

boliveira Jan 19, 2024 •

edited

Loading