Fetch multiple CMS template pages instead of just the first #2314
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, when trying to load contentTypes from the Headless CMS we just fetch the first page and ignore the hasNextPage response from the cmsClient. This PR changes that to load all publishedContentTypes before returning. This might be troublesome if a given account has thousands of content types of the same type (PLP, for instance). We can either increase the page size to make fewer requests or investigate if we can move the filtering to the CMS Client.
gvc
requested review from
renatamottam and
lucasfp13
and removed request for
a team
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
eduardoformiga
approved these changes
May 14, 2024
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
eduardoformiga
approved these changes
May 16, 2024
There was a problem hiding this comment.
Nice improvements with cache and Promise.all, @gvc ! ⭐
the test need to be updated but I'll let it pre approved.
There was a problem hiding this comment.
I think you need to update the tests after update the function (https://github.com/vtex/faststore/actions/runs/9103665826/job/25025988818?pr=2314)
Comment on lines
+48
to
+58
| /* | ||
| * This in memory cache exists because for each page (think category or department) | ||
| * we are fetching all the pages of the same content type from the headless CMS to | ||
| * find the one that matches the slug. | ||
| * | ||
| * So instead of making multiple request for the Headless CMS API for each page we make | ||
| * one for each content-type and reuse the results for the next page. | ||
| * | ||
| * Since we rebuild on a CMS publication the server will go away and will "invalidate" | ||
| * the cache | ||
| */ |
There was a problem hiding this comment.
💯
From my understanding, for each kind of PLP, the cache will be leveraged, right? 10/10
eduardoformiga
added a commit
that referenced
this pull request
May 24, 2024
## What's the purpose of this pull request? Reverts the cache approach when fetching multiple hCMS pages. It looks like the cache strategy used in this PR - #2314 It is bringing some issues for clients. So This PR aims to revert this logic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's the purpose of this pull request?
Currently, when trying to load contentTypes from the Headless CMS we just fetch the first page and ignore the hasNextPage response from the cmsClient.
How it works?
This PR changes that to load all publishedContentTypes before returning. This might be troublesome if a given account has thousands of content types of the same type (PLP, for instance). We can either increase the page size to make fewer requests or investigate if we can move the filtering to the CMS Client.
How to test it?
I've added tests, but this can be replicated by copying the
server/cms/index.tscode to a store's node_modules folder and you should see the effect.