From 152266ff269c381e3f62452503d36d41f09997b0 Mon Sep 17 00:00:00 2001 From: Tom Meyer Date: Wed, 25 Oct 2023 12:57:47 -0400 Subject: [PATCH] Ensure `_client` and `_server` Templates Don't Appear in Studio. (#423) This PR ensures Studio will not display the `_client.tsx` or `_server.tsx` Templates. These Templates are reserved files in PagesJS. They don't actually correspond to a Page, but instead dictate how SSR works. In the future, we will be able to exclude these files by supporting Globs in the Studio Config's paths. For now, Product does not have the appetite for adding Globs. We now prevent someone from adding a Page in Studio whose filename is `_client` or `_server` (assuming they're in PagesJS mode). J=SLAP-2960 TEST=auto, manual --- .../src/orchestrators/ParsingOrchestrator.ts | 12 ++++++++++++ .../orchestrators/createFilenameMapping.ts | 7 +++++-- .../ParsingOrchestrator/src/pages/_client.tsx | 12 ++++++++++++ .../ParsingOrchestrator/src/pages/_server.tsx | 19 +++++++++++++++++++ .../orchestrators/ParsingOrchestrator.test.ts | 6 +++++- .../studio-ui/src/utils/PageDataValidator.ts | 7 +++++++ .../tests/utils/PageDataValidator.test.ts | 19 +++++++++++++++---- 7 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_client.tsx create mode 100644 packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_server.tsx diff --git a/packages/studio-plugin/src/orchestrators/ParsingOrchestrator.ts b/packages/studio-plugin/src/orchestrators/ParsingOrchestrator.ts index 91d44e6fc..cc0a977ce 100644 --- a/packages/studio-plugin/src/orchestrators/ParsingOrchestrator.ts +++ b/packages/studio-plugin/src/orchestrators/ParsingOrchestrator.ts @@ -214,6 +214,18 @@ export default class ParsingOrchestrator { `The pages directory does not exist, expected directory to be at "${this.paths.pages}".` ); } + + if (this.studioConfig.isPagesJSRepo) { + const excludeReservedPagesJSFiles = (filename: string) => + !(filename.includes("_server") || filename.includes("_client")); + + return createFilenameMapping( + this.paths.pages, + this.getOrCreatePageFile, + excludeReservedPagesJSFiles + ); + } + return createFilenameMapping(this.paths.pages, this.getOrCreatePageFile); } diff --git a/packages/studio-plugin/src/orchestrators/createFilenameMapping.ts b/packages/studio-plugin/src/orchestrators/createFilenameMapping.ts index 8eea2eb00..704565117 100644 --- a/packages/studio-plugin/src/orchestrators/createFilenameMapping.ts +++ b/packages/studio-plugin/src/orchestrators/createFilenameMapping.ts @@ -3,11 +3,14 @@ import upath from "upath"; export default function createFilenameMapping( dirPath: string, - getMappedValue: (name: string) => T + getMappedValue: (name: string) => T, + fileFilter?: (filename: string) => boolean ): Record { const files = fs.readdirSync(dirPath, "utf-8").filter((filename) => { const absPath = upath.join(dirPath, filename); - return fs.lstatSync(absPath).isFile(); + + const isFile = fs.lstatSync(absPath).isFile(); + return fileFilter ? isFile && fileFilter(filename) : isFile; }); return files.reduce((filepathMapping, filename) => { const name = upath.basename(filename, ".tsx"); diff --git a/packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_client.tsx b/packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_client.tsx new file mode 100644 index 000000000..4ed088a76 --- /dev/null +++ b/packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_client.tsx @@ -0,0 +1,12 @@ +import { PageContext } from "@yext/pages"; +import { hydrateRoot } from "react-dom/client"; + +export { render }; + +const render = async (pageContext: PageContext) => { + const { Page, pageProps } = pageContext; + const rootElement = document.getElementById("reactele"); + if (rootElement) { + hydrateRoot(rootElement, ); + } +}; diff --git a/packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_server.tsx b/packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_server.tsx new file mode 100644 index 000000000..799f01df0 --- /dev/null +++ b/packages/studio-plugin/tests/__fixtures__/ParsingOrchestrator/src/pages/_server.tsx @@ -0,0 +1,19 @@ +import * as ReactDOMServer from "react-dom/server"; +import { PageContext } from "@yext/pages"; + +export { render }; + +const render = async (pageContext: PageContext) => { + const { Page, pageProps } = pageContext; + const viewHtml = ReactDOMServer.renderToString(); + return ` + + + + + + +
${viewHtml}
+ + `; +}; diff --git a/packages/studio-plugin/tests/orchestrators/ParsingOrchestrator.test.ts b/packages/studio-plugin/tests/orchestrators/ParsingOrchestrator.test.ts index 9f596f408..42e981511 100644 --- a/packages/studio-plugin/tests/orchestrators/ParsingOrchestrator.test.ts +++ b/packages/studio-plugin/tests/orchestrators/ParsingOrchestrator.test.ts @@ -111,6 +111,7 @@ describe("aggregates data as expected", () => { isPagesJS: true, }); const studioData = orchestrator.getStudioData(); + expect(studioData.pageNameToErrorPageState).toEqual({}); expect(studioData.pageNameToPageState).toEqual({ basicPage: { ...basicPageState, @@ -130,7 +131,10 @@ it("throws an error when the page imports components from unexpected folders", ( __dirname, "../__fixtures__/ParsingOrchestrator/src/pages" ); - createParsingOrchestrator({ paths: userPaths }).getStudioData(); + createParsingOrchestrator({ + paths: userPaths, + isPagesJS: true, + }).getStudioData(); expect(prettyPrintError).toHaveBeenCalledTimes(1); expect(prettyPrintError).toHaveBeenCalledWith( expect.stringMatching(/^Failed to parse PageState/), diff --git a/packages/studio-ui/src/utils/PageDataValidator.ts b/packages/studio-ui/src/utils/PageDataValidator.ts index 14d5eebca..531ae3ede 100644 --- a/packages/studio-ui/src/utils/PageDataValidator.ts +++ b/packages/studio-ui/src/utils/PageDataValidator.ts @@ -1,5 +1,7 @@ import { PagesRecord } from "../store/models/slices/PageSlice"; +export const PAGES_JS_RESERVED_PAGE_NAMES = ["_server", "_client"]; + export interface ValidationResult { valid: boolean; errorMessages: string[]; @@ -76,6 +78,11 @@ export default class PageDataValidator { if (pageName.length > 255) { errorMessages.push("Page name must be 255 characters or less."); } + if (this.isPagesJSRepo && PAGES_JS_RESERVED_PAGE_NAMES.includes(pageName)) { + errorMessages.push( + `Page name "${pageName}" is a reserved PagesJS filename.` + ); + } if (this.pages[pageName]) { errorMessages.push(`Page name "${pageName}" is already used.`); } diff --git a/packages/studio-ui/tests/utils/PageDataValidator.test.ts b/packages/studio-ui/tests/utils/PageDataValidator.test.ts index a0e3d584f..5aad5cf63 100644 --- a/packages/studio-ui/tests/utils/PageDataValidator.test.ts +++ b/packages/studio-ui/tests/utils/PageDataValidator.test.ts @@ -1,4 +1,5 @@ import PageDataValidator, { + PAGES_JS_RESERVED_PAGE_NAMES, ValidationResult, } from "../../src/utils/PageDataValidator"; @@ -44,11 +45,12 @@ describe("URL slug validation", () => { }); describe("page name validation", () => { + const validator = new PageDataValidator({ + isEntityPage: false, + isPagesJSRepo: true, + }); + function expectPageNameError(input: string, errorMessages: string[]) { - const validator = new PageDataValidator({ - isEntityPage: false, - isPagesJSRepo: true, - }); const result: ValidationResult = validator.validate({ pageName: input }); expect(result.valid).toBeFalsy(); expect(result.errorMessages).toEqual(expect.arrayContaining(errorMessages)); @@ -74,4 +76,13 @@ describe("page name validation", () => { const errorMessage = "Page name must be 255 characters or less."; expectPageNameError(longName, [errorMessage]); }); + + it("gives an error when using PagesJS Reserved Filenames", () => { + const checkReservedName = (pageName: string) => { + const errorMessage = `Page name "${pageName}" is a reserved PagesJS filename.`; + expectPageNameError(pageName, [errorMessage]); + }; + + PAGES_JS_RESERVED_PAGE_NAMES.forEach((name) => checkReservedName(name)); + }); });