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(dev/plugin): add localization to static pages #430

Merged
merged 15 commits into from
Nov 10, 2023

Conversation

alextaing
Copy link
Contributor

@alextaing alextaing commented Nov 8, 2023

This PR adds localization to static pages and adds an interface for StaticTemplateConfig. Localization can now be specified for static pages by including the following in the TemplateConfig.

export const config: StaticTemplateConfig = {
  name: "robots",
  locales: ["en", "es"]
};

This config is included in the templates.json/features.json to be consumed by Spruce as:

{
  "features": [
    {
      "name": "robots",
      "templateType": "JS",
      "staticPage": {
        "locales": ["en", "es"]
      }
    },
  ],
  "streams": []
}

Additionally, it was found that since static pages no longer have a single staticURL per featureName, the localDataManifest had to be refactored. This is because static pages now accept a document in their getPath function, meaning that when static pages may have different URLs depending on the document if the path uses the document's locale.

Furthermore, it was found that when running yext pages render to generate pages, static pages with a constant getPath return value multiple locales would only generate one static page. To deal with this behavior, pages will now throw an error when users have multiple static pages with identical paths.

J=SUMO-5451
TEST=manual

It was tested that you can specify locales in the TemplateConfig for static pages, and that they property show up in the templates.json. It was also tested that when yext pages generate-test-data is called, it generates a document for each locale specified in a static page.

@alextaing alextaing added the WIP label Nov 8, 2023
@alextaing alextaing changed the title Add Localization to Static Pages fix: add localization to static pages Nov 8, 2023
@alextaing alextaing changed the title fix: add localization to static pages feat(dev/plugin): add localization to static pages Nov 8, 2023
@alextaing alextaing removed the WIP label Nov 8, 2023
@alextaing alextaing marked this pull request as ready for review November 8, 2023 21:57
@alextaing alextaing requested a review from a team as a code owner November 8, 2023 21:57
@alextaing
Copy link
Contributor Author

alextaing commented Nov 8, 2023

@mkilpatrick One thing of note that I ran into when testing: If we have a static page with more than one locale, but a constant getPath return value:

export const config: StaticTemplateConfig = {
  name: "robots",
  locales: ['en', 'zh']
};
export const getPath = () => {
  return `robots.txt`;
};

In dev, it creates two pages: robots.txt?locale=en and robots.txt?locale=zh. But, when we generate pages using yext pages render, it only has one page , robots.txt and is filled with the contents of the page with the en locale.

Is this something we want to address as a part of this item, or something that we should make an item for? As of now, I'm not sure whether we should expect one of the locales to be overwritten, or if it should have two pages as we do in dev.

Copy link
Contributor

@asanehisa asanehisa left a comment

Choose a reason for hiding this comment

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

lgtm

packages/pages/src/common/src/template/types.ts Outdated Show resolved Hide resolved
@mkilpatrick
Copy link
Collaborator

@mkilpatrick One thing of note that I ran into when testing: If we have a static page with more than one locale, but a constant getPath return value:

export const config: StaticTemplateConfig = {
  name: "robots",
  locales: ['en', 'zh']
};
export const getPath = () => {
  return `robots.txt`;
};

In dev, it creates two pages: robots.txt?locale=en and robots.txt?locale=zh. But, when we generate pages using yext pages render, it only has one page , robots.txt and is filled with the contents of the page with the en locale.

Is this something we want to address as a part of this item, or something that we should make an item for? As of now, I'm not sure whether we should expect one of the locales to be overwritten, or if it should have two pages as we do in dev.

This is a good question. Could you bring it up in the original Slack thread discussion? Perhaps we should instead throw an error that you've defined multiple locales but only one output path/html file can be created.

Copy link
Collaborator

@mkilpatrick mkilpatrick left a comment

Choose a reason for hiding this comment

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

🔥

@alextaing alextaing merged commit 9d241f8 into main Nov 10, 2023
14 checks passed
@alextaing alextaing deleted the dev/localization-static-pages branch November 10, 2023 20:48
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