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

fix(dev): include document support for static page getPath functions. #421

Merged
merged 7 commits into from
Nov 3, 2023

Conversation

alextaing
Copy link
Contributor

@alextaing alextaing commented Nov 3, 2023

Fixes #415.

In the past, if users tried to access document fields in a static page getPath function, it would show up as "undefined" in the page URL. This PR addresses this by giving static pages access to the document object.

A couple changes were made to accommodate this functionality. We want to pass in a document in order correctly compute a getPath function. This means that when we iterate through templates to find the corresponding template document to our URL, (see findMatchingStaticTemplate), we need to retrieve the documents which is an asynchronous action. Therefore, the criterion function in findTemplateModuleInternal was changed to accept async functions.

For clarity, urlToFeature was also refactored. It was also found that defaultLocale was not being used in serverRenderRoute. Before, we hard-coded en to be the default Locale in urlToFeature. But in this refactor, we use defaultLocale instead.

J=SUMO-5417
TEST=manual

@alextaing alextaing added the WIP label Nov 3, 2023
@alextaing alextaing requested a review from a team as a code owner November 3, 2023 12:36
@alextaing alextaing changed the title Add Document support for Static Page getPath Functions. feat(dev): add document support for static page getPath functions. Nov 3, 2023
@alextaing alextaing removed the WIP label Nov 3, 2023
@mkilpatrick
Copy link
Collaborator

This is a "fix" vs a "feat". Can you update the commit message? Also mention "fixes [link to issue]" so it auto closes when this is merged.

Also does the document get injected during prod?

@alextaing alextaing changed the title feat(dev): add document support for static page getPath functions. fix(dev): add document support for static page getPath functions. Nov 3, 2023
@alextaing
Copy link
Contributor Author

This is a "fix" vs a "feat". Can you update the commit message? Also mention "fixes [link to issue]" so it auto closes when this is merged.

Also does the document get injected during prod?

Based on what I tested, when we create a getPath function that accesses the document object in a static page, then build and render the site using yext pages build, yext pages render, etc, the getPath functions correctly access the document fields. This led me to believe that the document also gets injected during this flow. Is this what you mean by prod, or is there another flow?

export const getPath: GetPath<ExternalImageData> = ({document}) => {
  return `${document.locale}.html`;
};
image

@alextaing alextaing changed the title fix(dev): add document support for static page getPath functions. fix(dev): include document support for static page getPath functions. Nov 3, 2023
@alextaing alextaing merged commit c7ae21a into main Nov 3, 2023
16 checks passed
@alextaing alextaing deleted the dev/static-page-get-path-issues branch November 3, 2023 17:54
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.

Issues using document data in getPath for static features locally
2 participants