From e2fb59bd1d6dde3f135cdf264a9f51d89b001d80 Mon Sep 17 00:00:00 2001 From: kiliman Date: Fri, 1 Apr 2022 16:38:12 -0400 Subject: [PATCH 01/17] feat(remix-react): Add array-syntax for `meta` export --- integration/meta-test.ts | 115 ++++++++++++++++++ packages/remix-react/__tests__/meta-test.tsx | 74 +++++++++++ packages/remix-react/components.tsx | 84 ++++++++++++- packages/remix-react/routeModules.ts | 2 +- packages/remix-server-runtime/routeModules.ts | 2 +- 5 files changed, 269 insertions(+), 8 deletions(-) create mode 100644 packages/remix-react/__tests__/meta-test.tsx diff --git a/integration/meta-test.ts b/integration/meta-test.ts index efe0a6569a4..e18a9e6277e 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -119,3 +119,118 @@ test.describe("meta", () => { ).toBeTruthy(); }); }); + +describe("meta array syntax", () => { + let fixture: Fixture; + let app: AppFixture; + + beforeAll(async () => { + fixture = await createFixture({ + files: { + "app/root.jsx": js` + import { json, Meta, Links, Outlet, Scripts } from "remix"; + + export const loader = async () => + json({ + description: "This is a meta page", + title: "Meta Page", + }); + + export const meta = ({ data }) => [ + { charset: "utf-8" }, + { description: data.description }, + { "og:image": "https://picsum.photos/200/200" }, + { "og:type": data.contentType }, // undefined + { httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, + { title: data.title }, + ]; + + export default function Root() { + return ( + + + + + + + + + + + ); + } + `, + + "app/routes/index.jsx": js` + export default function Index() { + return
This is the index file
; + } + `, + }, + }); + + app = await createAppFixture(fixture); + }); + + afterAll(async () => await app.close()); + + test("empty meta does not render a tag", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( + 'No element matches selector "meta[property="og:type"]"' + ); + await enableJavaScript(); + }); + + test("meta { charset } adds a ", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { title } adds a ", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml("title")).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { 'og:*' } adds a <meta property='og:*' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { description } adds a <meta name='description' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { refresh } adds a <meta http-equiv='refresh' content='3;url=https://www.mozilla.org' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect( + await app.getHtml( + 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' + ) + ).toBeTruthy(); + await enableJavaScript(); + }); +}); diff --git a/packages/remix-react/__tests__/meta-test.tsx b/packages/remix-react/__tests__/meta-test.tsx new file mode 100644 index 00000000000..c65b1b02440 --- /dev/null +++ b/packages/remix-react/__tests__/meta-test.tsx @@ -0,0 +1,74 @@ +import "@testing-library/jest-dom/extend-expect"; +import { processMeta } from "../components"; +import { HtmlMetaDescriptor, MetaFunction } from "../routeModules"; + +describe("meta", () => { + it.only(`renders proper <meta> tags`, () => { + function meta({ data }): HtmlMetaDescriptor { + return { + charset: "utf-8", + description: data.description, + "og:image": "https://picsum.photos/200/200", + "og:type": data.contentType, // undefined + refresh: { + httpEquiv: "refresh", + content: "3;url=https://www.mozilla.org", + }, + title: data.title, + }; + } + function meta2({ data }): HtmlMetaDescriptor[] { + return [ + { name: "description", content: "override description" }, + { property: "og:image", content: "https://remix.run/logo.png" }, + { property: "og:type", content: "image/png" }, + { + httpEquiv: "refresh", + content: "5;url=https://google.com", + }, + { title: "Updated title" }, + { name: "viewport", content: "width=device-width, initial-scale=1" }, + ]; + } + + const result = getMeta( + { + title: "test title", + description: "test description", + }, + [meta, meta2] + ); + + // title should override the title from the first meta function + let titleMeta = result.find((meta) => meta.hasOwnProperty("title")); + expect(titleMeta["title"]).toBe("Updated title"); + // viewport should be added + let viewportMeta = result.find((meta) => meta["name"] === "viewport"); + expect(viewportMeta["content"]).toBe("width=device-width, initial-scale=1"); + }); +}); + +function getMeta(data: any, metaFunctions: MetaFunction[]) { + let meta: HtmlMetaDescriptor[] = []; + metaFunctions.forEach((metaFunction) => { + let routeMeta = metaFunction({ + data, + parentsData: {}, + params: {}, + location: null, + }); + if (routeMeta) { + meta = processMeta(meta, routeMeta); + } + }); + + return meta; +} + +function renderMeta(meta: HtmlMetaDescriptor[]) { + return meta.map((metaDescriptor) => { + return `<meta ${Object.keys(metaDescriptor) + .map((key) => `${key}="${metaDescriptor[key]}"`) + .join(" ")} />`; + }); +} diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 0b39228f3b4..a05eaf8f36d 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -697,7 +697,7 @@ export function Meta() { let { matches, routeData, routeModules } = useRemixEntryContext(); let location = useLocation(); - let meta: HtmlMetaDescriptor = {}; + let meta: HtmlMetaDescriptor[] = []; let parentsData: { [routeId: string]: AppData } = {}; for (let match of matches) { @@ -712,7 +712,9 @@ export function Meta() { typeof routeModule.meta === "function" ? routeModule.meta({ data, parentsData, params, location }) : routeModule.meta; - Object.assign(meta, routeMeta); + if (routeMeta) { + processMeta(meta, routeMeta); + } } parentsData[routeId] = data; @@ -720,13 +722,13 @@ export function Meta() { return ( <> - {Object.entries(meta).map(([name, value]) => { - if (!value) { + {meta.map((metaDescriptor) => { + if (!metaDescriptor) { return null; } - if (["charset", "charSet"].includes(name)) { - return <meta key="charset" charSet={value as string} />; + if (metaDescriptor.hasOwnProperty("charset")) { + return <meta key="charSet" charSet={metaDescriptor.charset} />; } if (name === "title") { @@ -758,6 +760,76 @@ export function Meta() { ); } +/* + * This function processes a meta descriptor and adds it to the meta array. + * This converts the original object syntax to new array syntax. + */ +export function processMeta( + meta: HtmlMetaDescriptor[], + routeMeta: HtmlMetaDescriptor | HtmlMetaDescriptor[] +) { + if (!Array.isArray(routeMeta)) { + // normalize to explicit objects + let metaArray = Object.entries(routeMeta).map(([key, value]) => { + if (!value) return null; + if (["charSet", "charset"].includes(key)) { + return { charSet: value }; + } + if (key === "title") { + return { title: value }; + } + if (key.startsWith("og:")) { + // set key here to help with merging so don't have to set explicit key + return { key, property: key, content: value }; + } + if (typeof value === "string") { + return { name: key, content: value }; + } + // this is the object version so just return it + return value; + }); + // routeMeta has been converted to an array of objects + // ts-ignore + routeMeta = metaArray as HtmlMetaDescriptor[]; + } + // remove nulls | undefined + routeMeta = routeMeta.filter(Boolean); + // first time through so no need to merge + if (!meta.length) { + return routeMeta; + } + // merge routeMeta into meta array + // child meta takes precedence over parent meta + routeMeta.forEach((metaDescriptor) => { + // find the index of the metaDescriptor in the meta array with same key + for (let key of ["key", "title", "name", "property", "httpEquiv"]) { + if (metaDescriptor.hasOwnProperty(key)) { + let index = -1; + if (key === "title") { + index = meta.findIndex((m) => m.hasOwnProperty("title")); + } else { + if (key === "key") { + // key value refers to name of key in metaDescriptor + key = metaDescriptor["key"] as string; + } + index = meta.findIndex((m) => m[key] === metaDescriptor[key]); + } + if (index >= 0) { + // found a match so replace the metaDescriptor with the one from routeMeta + meta[index] = metaDescriptor; + } else { + meta.push(metaDescriptor); + } + return; + } + } + // no keys found, warn user and add to meta array + console.warn(`No key found for meta ${JSON.stringify(metaDescriptor)}`); + meta.push(metaDescriptor); + }); + return meta; +} + /** * Tracks whether Remix has finished hydrating or not, so scripts can be skipped * during client-side updates. diff --git a/packages/remix-react/routeModules.ts b/packages/remix-react/routeModules.ts index ab163a7ddff..66d31f3ae1a 100644 --- a/packages/remix-react/routeModules.ts +++ b/packages/remix-react/routeModules.ts @@ -61,7 +61,7 @@ export interface MetaFunction { parentsData: RouteData; params: Params; location: Location; - }): HtmlMetaDescriptor | undefined; + }): HtmlMetaDescriptor | HtmlMetaDescriptor[] | undefined; } /** diff --git a/packages/remix-server-runtime/routeModules.ts b/packages/remix-server-runtime/routeModules.ts index ec228386866..18b38fb3f37 100644 --- a/packages/remix-server-runtime/routeModules.ts +++ b/packages/remix-server-runtime/routeModules.ts @@ -142,7 +142,7 @@ export interface MetaFunction< } & RouteData; params: Params; location: Location; - }): HtmlMetaDescriptor; + }): HtmlMetaDescriptor | HtmlMetaDescriptor[]; } /** From b1e9a25b6cdb42d4408a34d7d8b8a46dc7dc0688 Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 16:45:37 -0400 Subject: [PATCH 02/17] chore: remove .only from test --- packages/remix-react/__tests__/meta-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/remix-react/__tests__/meta-test.tsx b/packages/remix-react/__tests__/meta-test.tsx index c65b1b02440..0f066c98c54 100644 --- a/packages/remix-react/__tests__/meta-test.tsx +++ b/packages/remix-react/__tests__/meta-test.tsx @@ -3,7 +3,7 @@ import { processMeta } from "../components"; import { HtmlMetaDescriptor, MetaFunction } from "../routeModules"; describe("meta", () => { - it.only(`renders proper <meta> tags`, () => { + it(`renders proper <meta> tags`, () => { function meta({ data }): HtmlMetaDescriptor { return { charset: "utf-8", From 9ca21f696da5a43f8ed916f75d0a5c1455c0d8fa Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 16:51:37 -0400 Subject: [PATCH 03/17] fix: Fix lint errors --- packages/remix-react/__tests__/meta-test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/remix-react/__tests__/meta-test.tsx b/packages/remix-react/__tests__/meta-test.tsx index 0f066c98c54..b3cf148b27a 100644 --- a/packages/remix-react/__tests__/meta-test.tsx +++ b/packages/remix-react/__tests__/meta-test.tsx @@ -1,6 +1,6 @@ import "@testing-library/jest-dom/extend-expect"; import { processMeta } from "../components"; -import { HtmlMetaDescriptor, MetaFunction } from "../routeModules"; +import type { HtmlMetaDescriptor, MetaFunction } from "../routeModules"; describe("meta", () => { it(`renders proper <meta> tags`, () => { @@ -31,7 +31,7 @@ describe("meta", () => { ]; } - const result = getMeta( + let result = getMeta( { title: "test title", description: "test description", @@ -65,6 +65,7 @@ function getMeta(data: any, metaFunctions: MetaFunction[]) { return meta; } +// eslint-disable-next-line @typescript-eslint/no-unused-vars function renderMeta(meta: HtmlMetaDescriptor[]) { return meta.map((metaDescriptor) => { return `<meta ${Object.keys(metaDescriptor) From c1ad9f884a63efa4fe54414f624f3dec9faee945 Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 17:28:09 -0400 Subject: [PATCH 04/17] fix: Revert meta-test back to original --- integration/meta-test.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index e18a9e6277e..de4ed980f26 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -136,14 +136,17 @@ describe("meta array syntax", () => { title: "Meta Page", }); - export const meta = ({ data }) => [ - { charset: "utf-8" }, - { description: data.description }, - { "og:image": "https://picsum.photos/200/200" }, - { "og:type": data.contentType }, // undefined - { httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, - { title: data.title }, - ]; + export const meta = ({ data }) => ({ + charset: "utf-8", + description: data.description, + "og:image": "https://picsum.photos/200/200", + "og:type": data.contentType, // undefined + refresh: { + httpEquiv: "refresh", + content: "3;url=https://www.mozilla.org", + }, + title: data.title, + }); export default function Root() { return ( From eae5a201b36ddb91d749280e5bd2c807d3eb4b5d Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 18:27:50 -0400 Subject: [PATCH 05/17] fix: update meta from processMeta() --- packages/remix-react/components.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index a05eaf8f36d..e8a263a4787 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -713,7 +713,7 @@ export function Meta() { ? routeModule.meta({ data, parentsData, params, location }) : routeModule.meta; if (routeMeta) { - processMeta(meta, routeMeta); + meta = processMeta(meta, routeMeta); } } From 8eda69ef5a7b563e6379d9a7c7912cb991c26c19 Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 18:43:18 -0400 Subject: [PATCH 06/17] fix: Re-add the array-syntax test --- integration/meta-test.ts | 115 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index de4ed980f26..4e5253db31c 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -237,3 +237,118 @@ describe("meta array syntax", () => { await enableJavaScript(); }); }); + +describe("meta array-syntax", () => { + let fixture: Fixture; + let app: AppFixture; + + beforeAll(async () => { + fixture = await createFixture({ + files: { + "app/root.jsx": js` + import { json, Meta, Links, Outlet, Scripts } from "remix"; + + export const loader = async () => + json({ + description: "This is a meta page", + title: "Meta Page", + }); + + export const meta = ({ data }) => [ + { charset: "utf-8" }, + { description: data.description }, + { "og:image": "https://picsum.photos/200/200" }, + { "og:type": data.contentType }, // undefined + { httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, + { title: data.title }, + ]; + + export default function Root() { + return ( + <html lang="en"> + <head> + <Meta /> + <Links /> + </head> + <body> + <Outlet /> + <Scripts /> + </body> + </html> + ); + } + `, + + "app/routes/index.jsx": js` + export default function Index() { + return <div>This is the index file</div>; + } + `, + }, + }); + + app = await createAppFixture(fixture); + }); + + afterAll(async () => await app.close()); + + test("empty meta does not render a tag", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( + 'No element matches selector "meta[property="og:type"]"' + ); + await enableJavaScript(); + }); + + test("meta { charset } adds a <meta charset='utf-8' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { title } adds a <title />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml("title")).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { 'og:*' } adds a <meta property='og:*' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { description } adds a <meta name='description' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { refresh } adds a <meta http-equiv='refresh' content='3;url=https://www.mozilla.org' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect( + await app.getHtml( + 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' + ) + ).toBeTruthy(); + await enableJavaScript(); + }); +}); From 90eb09a59bb848b7a0eac499373788bcc82276f6 Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 18:52:07 -0400 Subject: [PATCH 07/17] fix: Remove object-syntax again --- integration/meta-test.ts | 214 +++++++++++++++++++-------------------- 1 file changed, 107 insertions(+), 107 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index 4e5253db31c..c575eac9c58 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -130,113 +130,113 @@ describe("meta array syntax", () => { "app/root.jsx": js` import { json, Meta, Links, Outlet, Scripts } from "remix"; - export const loader = async () => - json({ - description: "This is a meta page", - title: "Meta Page", - }); - - export const meta = ({ data }) => ({ - charset: "utf-8", - description: data.description, - "og:image": "https://picsum.photos/200/200", - "og:type": data.contentType, // undefined - refresh: { - httpEquiv: "refresh", - content: "3;url=https://www.mozilla.org", - }, - title: data.title, - }); - - export default function Root() { - return ( - <html lang="en"> - <head> - <Meta /> - <Links /> - </head> - <body> - <Outlet /> - <Scripts /> - </body> - </html> - ); - } - `, - - "app/routes/index.jsx": js` - export default function Index() { - return <div>This is the index file</div>; - } - `, - }, - }); - - app = await createAppFixture(fixture); - }); - - afterAll(async () => await app.close()); - - test("empty meta does not render a tag", async () => { - let enableJavaScript = await app.disableJavaScript(); - - await app.goto("/"); - - await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( - 'No element matches selector "meta[property="og:type"]"' - ); - await enableJavaScript(); - }); - - test("meta { charset } adds a <meta charset='utf-8' />", async () => { - let enableJavaScript = await app.disableJavaScript(); - - await app.goto("/"); - - expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); - await enableJavaScript(); - }); - - test("meta { title } adds a <title />", async () => { - let enableJavaScript = await app.disableJavaScript(); - - await app.goto("/"); - - expect(await app.getHtml("title")).toBeTruthy(); - await enableJavaScript(); - }); - - test("meta { 'og:*' } adds a <meta property='og:*' />", async () => { - let enableJavaScript = await app.disableJavaScript(); - - await app.goto("/"); - - expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); - await enableJavaScript(); - }); - - test("meta { description } adds a <meta name='description' />", async () => { - let enableJavaScript = await app.disableJavaScript(); - - await app.goto("/"); - - expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); - await enableJavaScript(); - }); - - test("meta { refresh } adds a <meta http-equiv='refresh' content='3;url=https://www.mozilla.org' />", async () => { - let enableJavaScript = await app.disableJavaScript(); - - await app.goto("/"); - - expect( - await app.getHtml( - 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' - ) - ).toBeTruthy(); - await enableJavaScript(); - }); -}); +// export const loader = async () => +// json({ +// description: "This is a meta page", +// title: "Meta Page", +// }); + +// export const meta = ({ data }) => ({ +// charset: "utf-8", +// description: data.description, +// "og:image": "https://picsum.photos/200/200", +// "og:type": data.contentType, // undefined +// refresh: { +// httpEquiv: "refresh", +// content: "3;url=https://www.mozilla.org", +// }, +// title: data.title, +// }); + +// export default function Root() { +// return ( +// <html lang="en"> +// <head> +// <Meta /> +// <Links /> +// </head> +// <body> +// <Outlet /> +// <Scripts /> +// </body> +// </html> +// ); +// } +// `, + +// "app/routes/index.jsx": js` +// export default function Index() { +// return <div>This is the index file</div>; +// } +// `, +// }, +// }); + +// app = await createAppFixture(fixture); +// }); + +// afterAll(async () => await app.close()); + +// test("empty meta does not render a tag", async () => { +// let enableJavaScript = await app.disableJavaScript(); + +// await app.goto("/"); + +// await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( +// 'No element matches selector "meta[property="og:type"]"' +// ); +// await enableJavaScript(); +// }); + +// test("meta { charset } adds a <meta charset='utf-8' />", async () => { +// let enableJavaScript = await app.disableJavaScript(); + +// await app.goto("/"); + +// expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); +// await enableJavaScript(); +// }); + +// test("meta { title } adds a <title />", async () => { +// let enableJavaScript = await app.disableJavaScript(); + +// await app.goto("/"); + +// expect(await app.getHtml("title")).toBeTruthy(); +// await enableJavaScript(); +// }); + +// test("meta { 'og:*' } adds a <meta property='og:*' />", async () => { +// let enableJavaScript = await app.disableJavaScript(); + +// await app.goto("/"); + +// expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); +// await enableJavaScript(); +// }); + +// test("meta { description } adds a <meta name='description' />", async () => { +// let enableJavaScript = await app.disableJavaScript(); + +// await app.goto("/"); + +// expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); +// await enableJavaScript(); +// }); + +// test("meta { refresh } adds a <meta http-equiv='refresh' content='3;url=https://www.mozilla.org' />", async () => { +// let enableJavaScript = await app.disableJavaScript(); + +// await app.goto("/"); + +// expect( +// await app.getHtml( +// 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' +// ) +// ).toBeTruthy(); +// await enableJavaScript(); +// }); +// }); describe("meta array-syntax", () => { let fixture: Fixture; From d383bd35b2baa17873365bcb6b37d616a282236a Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 18:59:46 -0400 Subject: [PATCH 08/17] fix: Use correct format for meta descriptors --- integration/meta-test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index c575eac9c58..b427d0f443f 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -256,9 +256,9 @@ describe("meta array-syntax", () => { export const meta = ({ data }) => [ { charset: "utf-8" }, - { description: data.description }, - { "og:image": "https://picsum.photos/200/200" }, - { "og:type": data.contentType }, // undefined + { name: "description", content: "data.description" }, + { property: "og:image", content: "https://picsum.photos/200/200" }, + { property: "og:type", content: data.contentType }, // undefined { httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, { title: data.title }, ]; From 7d62a6137a545dd3e142a33e1ca2e72f0812ce38 Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 19:24:49 -0400 Subject: [PATCH 09/17] fix: Argh... literally just trying a single test and render content --- integration/meta-test.ts | 80 +++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index b427d0f443f..31fa0d7d9d1 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -266,14 +266,7 @@ describe("meta array-syntax", () => { export default function Root() { return ( <html lang="en"> - <head> - <Meta /> - <Links /> - </head> - <body> - <Outlet /> - <Scripts /> - </body> + <Meta /> </html> ); } @@ -296,59 +289,60 @@ describe("meta array-syntax", () => { let enableJavaScript = await app.disableJavaScript(); await app.goto("/"); - + let html = await app.getHtml("html"); + console.log(html); await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( 'No element matches selector "meta[property="og:type"]"' ); await enableJavaScript(); }); - test("meta { charset } adds a <meta charset='utf-8' />", async () => { - let enableJavaScript = await app.disableJavaScript(); + // test("meta { charset } adds a <meta charset='utf-8' />", async () => { + // let enableJavaScript = await app.disableJavaScript(); - await app.goto("/"); + // await app.goto("/"); - expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); - await enableJavaScript(); - }); + // expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); + // await enableJavaScript(); + // }); - test("meta { title } adds a <title />", async () => { - let enableJavaScript = await app.disableJavaScript(); + // test("meta { title } adds a <title />", async () => { + // let enableJavaScript = await app.disableJavaScript(); - await app.goto("/"); + // await app.goto("/"); - expect(await app.getHtml("title")).toBeTruthy(); - await enableJavaScript(); - }); + // expect(await app.getHtml("title")).toBeTruthy(); + // await enableJavaScript(); + // }); - test("meta { 'og:*' } adds a <meta property='og:*' />", async () => { - let enableJavaScript = await app.disableJavaScript(); + // test("meta { 'og:*' } adds a <meta property='og:*' />", async () => { + // let enableJavaScript = await app.disableJavaScript(); - await app.goto("/"); + // await app.goto("/"); - expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); - await enableJavaScript(); - }); + // expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); + // await enableJavaScript(); + // }); - test("meta { description } adds a <meta name='description' />", async () => { - let enableJavaScript = await app.disableJavaScript(); + // test("meta { description } adds a <meta name='description' />", async () => { + // let enableJavaScript = await app.disableJavaScript(); - await app.goto("/"); + // await app.goto("/"); - expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); - await enableJavaScript(); - }); + // expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); + // await enableJavaScript(); + // }); - test("meta { refresh } adds a <meta http-equiv='refresh' content='3;url=https://www.mozilla.org' />", async () => { - let enableJavaScript = await app.disableJavaScript(); + // test("meta { refresh } adds a <meta http-equiv='refresh' content='3;url=https://www.mozilla.org' />", async () => { + // let enableJavaScript = await app.disableJavaScript(); - await app.goto("/"); + // await app.goto("/"); - expect( - await app.getHtml( - 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' - ) - ).toBeTruthy(); - await enableJavaScript(); - }); + // expect( + // await app.getHtml( + // 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' + // ) + // ).toBeTruthy(); + // await enableJavaScript(); + // }); }); From cf71b5263fff132e128a6aa60a879326c0e3ebcb Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Fri, 1 Apr 2022 19:31:30 -0400 Subject: [PATCH 10/17] fix: These tests are killing me --- integration/meta-test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index 31fa0d7d9d1..e98f4d2bd9d 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -266,7 +266,9 @@ describe("meta array-syntax", () => { export default function Root() { return ( <html lang="en"> + <head> <Meta /> + </head> </html> ); } @@ -289,8 +291,8 @@ describe("meta array-syntax", () => { let enableJavaScript = await app.disableJavaScript(); await app.goto("/"); - let html = await app.getHtml("html"); - console.log(html); + let head = await app.getHtml("head"); + console.log(head); await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( 'No element matches selector "meta[property="og:type"]"' ); From 8a81d93c54c94a084d9c2205125ae2deae183dd4 Mon Sep 17 00:00:00 2001 From: kiliman <kiliman@gmail.com> Date: Sat, 2 Apr 2022 12:57:10 -0400 Subject: [PATCH 11/17] fix: Update HtmlMetaDescriptor type and use Map to manage meta --- integration/meta-test.ts | 8 +- packages/remix-react/__tests__/meta-test.tsx | 53 +++++--- packages/remix-react/components.tsx | 127 +++++++++---------- packages/remix-react/routeModules.ts | 16 +-- 4 files changed, 103 insertions(+), 101 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index e98f4d2bd9d..0583ee2ae49 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -255,12 +255,12 @@ describe("meta array-syntax", () => { }); export const meta = ({ data }) => [ - { charset: "utf-8" }, - { name: "description", content: "data.description" }, + { key: "charset", content: "utf-8" }, + { name: "description", content: data.description }, { property: "og:image", content: "https://picsum.photos/200/200" }, { property: "og:type", content: data.contentType }, // undefined - { httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, - { title: data.title }, + { key: "http-equiv:refresh", httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, + { key: "title", content: data.title }, ]; export default function Root() { diff --git a/packages/remix-react/__tests__/meta-test.tsx b/packages/remix-react/__tests__/meta-test.tsx index b3cf148b27a..f6d3b430350 100644 --- a/packages/remix-react/__tests__/meta-test.tsx +++ b/packages/remix-react/__tests__/meta-test.tsx @@ -1,6 +1,8 @@ +import React from "react"; import "@testing-library/jest-dom/extend-expect"; import { processMeta } from "../components"; import type { HtmlMetaDescriptor, MetaFunction } from "../routeModules"; +import { renderToString } from "react-dom/server"; describe("meta", () => { it(`renders proper <meta> tags`, () => { @@ -10,10 +12,6 @@ describe("meta", () => { description: data.description, "og:image": "https://picsum.photos/200/200", "og:type": data.contentType, // undefined - refresh: { - httpEquiv: "refresh", - content: "3;url=https://www.mozilla.org", - }, title: data.title, }; } @@ -23,15 +21,17 @@ describe("meta", () => { { property: "og:image", content: "https://remix.run/logo.png" }, { property: "og:type", content: "image/png" }, { + key: "http-equiv:refresh", httpEquiv: "refresh", content: "5;url=https://google.com", }, - { title: "Updated title" }, + { key: "title", content: "Updated title" }, + { key: "charset", content: "utf-16" }, { name: "viewport", content: "width=device-width, initial-scale=1" }, ]; } - let result = getMeta( + let map = getMeta( { title: "test title", description: "test description", @@ -39,17 +39,23 @@ describe("meta", () => { [meta, meta2] ); + // let rendered = renderMeta(map); + // let html = renderToString(rendered); + // console.log(html); + // title should override the title from the first meta function - let titleMeta = result.find((meta) => meta.hasOwnProperty("title")); - expect(titleMeta["title"]).toBe("Updated title"); + expect(map.get("title").content).toBe("Updated title"); // viewport should be added - let viewportMeta = result.find((meta) => meta["name"] === "viewport"); - expect(viewportMeta["content"]).toBe("width=device-width, initial-scale=1"); + expect(map.get("viewport").content).toBe( + "width=device-width, initial-scale=1" + ); }); }); +type MetaMap = Map<string, HtmlMetaDescriptor>; + function getMeta(data: any, metaFunctions: MetaFunction[]) { - let meta: HtmlMetaDescriptor[] = []; + let meta: MetaMap = new Map(); metaFunctions.forEach((metaFunction) => { let routeMeta = metaFunction({ data, @@ -58,18 +64,27 @@ function getMeta(data: any, metaFunctions: MetaFunction[]) { location: null, }); if (routeMeta) { - meta = processMeta(meta, routeMeta); + processMeta(meta, routeMeta); + //console.log(meta, routeMeta); } }); return meta; } -// eslint-disable-next-line @typescript-eslint/no-unused-vars -function renderMeta(meta: HtmlMetaDescriptor[]) { - return meta.map((metaDescriptor) => { - return `<meta ${Object.keys(metaDescriptor) - .map((key) => `${key}="${metaDescriptor[key]}"`) - .join(" ")} />`; - }); +/* eslint-disable-next-line @typescript-eslint/no-unused-vars */ +function renderMeta(meta: MetaMap) { + return ( + <> + {[...meta.entries()].map(([key, value]) => { + if (key === "title" && typeof value.content === "string") { + return <title key={key}>{value.content}; + } + if (key === "charset" && typeof value.content === "string") { + return ; + } + return ; + })} + + ); } diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index e8a263a4787..e013d6355a1 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -688,6 +688,8 @@ function PrefetchPageLinksImpl({ ); } +type MetaMap = Map; + /** * Renders the `` and `<meta>` tags for the current routes. * @@ -697,7 +699,7 @@ export function Meta() { let { matches, routeData, routeModules } = useRemixEntryContext(); let location = useLocation(); - let meta: HtmlMetaDescriptor[] = []; + let meta: MetaMap = new Map(); let parentsData: { [routeId: string]: AppData } = {}; for (let match of matches) { @@ -713,7 +715,7 @@ export function Meta() { ? routeModule.meta({ data, parentsData, params, location }) : routeModule.meta; if (routeMeta) { - meta = processMeta(meta, routeMeta); + processMeta(meta, routeMeta); } } @@ -722,13 +724,12 @@ export function Meta() { return ( <> - {meta.map((metaDescriptor) => { - if (!metaDescriptor) { - return null; + {[...meta.entries()].map(([key, value]) => { + if (key === "title" && typeof value.content === "string") { + return <title key={key}>{value.content}; } - - if (metaDescriptor.hasOwnProperty("charset")) { - return ; + if (key === "charset" && typeof value.content === "string") { + return ; } if (name === "title") { @@ -765,69 +766,59 @@ export function Meta() { * This converts the original object syntax to new array syntax. */ export function processMeta( - meta: HtmlMetaDescriptor[], + meta: MetaMap, routeMeta: HtmlMetaDescriptor | HtmlMetaDescriptor[] ) { - if (!Array.isArray(routeMeta)) { - // normalize to explicit objects - let metaArray = Object.entries(routeMeta).map(([key, value]) => { - if (!value) return null; - if (["charSet", "charset"].includes(key)) { - return { charSet: value }; - } - if (key === "title") { - return { title: value }; - } - if (key.startsWith("og:")) { - // set key here to help with merging so don't have to set explicit key - return { key, property: key, content: value }; - } - if (typeof value === "string") { - return { name: key, content: value }; - } - // this is the object version so just return it - return value; - }); - // routeMeta has been converted to an array of objects - // ts-ignore - routeMeta = metaArray as HtmlMetaDescriptor[]; - } - // remove nulls | undefined - routeMeta = routeMeta.filter(Boolean); - // first time through so no need to merge - if (!meta.length) { - return routeMeta; - } - // merge routeMeta into meta array - // child meta takes precedence over parent meta - routeMeta.forEach((metaDescriptor) => { - // find the index of the metaDescriptor in the meta array with same key - for (let key of ["key", "title", "name", "property", "httpEquiv"]) { - if (metaDescriptor.hasOwnProperty(key)) { - let index = -1; - if (key === "title") { - index = meta.findIndex((m) => m.hasOwnProperty("title")); - } else { - if (key === "key") { - // key value refers to name of key in metaDescriptor - key = metaDescriptor["key"] as string; - } - index = meta.findIndex((m) => m[key] === metaDescriptor[key]); - } - if (index >= 0) { - // found a match so replace the metaDescriptor with the one from routeMeta - meta[index] = metaDescriptor; - } else { - meta.push(metaDescriptor); - } - return; - } - } - // no keys found, warn user and add to meta array - console.warn(`No key found for meta ${JSON.stringify(metaDescriptor)}`); - meta.push(metaDescriptor); + let items: HtmlMetaDescriptor[] = Array.isArray(routeMeta) + ? routeMeta + : Object.entries(routeMeta) + .map(([key, value]) => { + if (!value) return []; + + let propertyName = key.startsWith("og:") ? "property" : "name"; + return key === "title" + ? { key: "title", content: assertString(value) } + : ["charset", "charSet"].includes(key) + ? { key: "charset", content: assertString(value) } + : Array.isArray(value) + ? value.map((content) => ({ + key: `${key}.${content}`, + [propertyName]: key, + content, + })) + : { key, [propertyName]: key, content: value }; + }) + .flat(); + + items.forEach((item) => { + let [key, value] = computeKey(item); + // child routes always override parent routes + meta.set(key, value); }); - return meta; +} + +function assertString(value: string | string[]): string { + if (typeof value !== "string") { + throw new Error("Expected string, got an array of strings"); + } + return value; +} + +function generateKey(item: HtmlMetaDescriptor) { + console.warn("No key provided to meta", item); + return JSON.stringify(item); // generate a reasonable key +} + +function computeKey(item: HtmlMetaDescriptor): [string, HtmlMetaDescriptor] { + let { + name, + property, + title, + key = name ?? property ?? title ?? generateKey(item), + ...rest + } = item; + + return [key, { name, property, title, ...rest }]; } /** diff --git a/packages/remix-react/routeModules.ts b/packages/remix-react/routeModules.ts index 66d31f3ae1a..277faf817c8 100644 --- a/packages/remix-react/routeModules.ts +++ b/packages/remix-react/routeModules.ts @@ -70,17 +70,13 @@ export interface MetaFunction { * tag, or an array of strings that will render multiple tags with the same * `name` attribute. */ -export interface HtmlMetaDescriptor { - charset?: "utf-8"; - charSet?: "utf-8"; +export type HtmlMetaDescriptor = { + key?: string; + name?: string; + property?: string; title?: string; - [name: string]: - | null - | string - | undefined - | Record - | Array | string>; -} + content?: string; +} & Record; /** * During client side transitions Remix will optimize reloading of routes that From 983fb9dafc2c204d71c12e04c2456eba48e8cbf6 Mon Sep 17 00:00:00 2001 From: kiliman Date: Sat, 2 Apr 2022 13:02:14 -0400 Subject: [PATCH 12/17] fix: Fix eslint errors --- packages/remix-react/__tests__/meta-test.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/remix-react/__tests__/meta-test.tsx b/packages/remix-react/__tests__/meta-test.tsx index f6d3b430350..7c773540000 100644 --- a/packages/remix-react/__tests__/meta-test.tsx +++ b/packages/remix-react/__tests__/meta-test.tsx @@ -1,8 +1,10 @@ import React from "react"; +/* eslint-disable @typescript-eslint/no-unused-vars */ +import { renderToString } from "react-dom/server"; import "@testing-library/jest-dom/extend-expect"; + import { processMeta } from "../components"; import type { HtmlMetaDescriptor, MetaFunction } from "../routeModules"; -import { renderToString } from "react-dom/server"; describe("meta", () => { it(`renders proper tags`, () => { From dc605b3070b8effa2ca5975dba238fa9c0daae0b Mon Sep 17 00:00:00 2001 From: kiliman Date: Wed, 6 Apr 2022 11:28:54 -0400 Subject: [PATCH 13/17] fix: Filter undefined and support title prop --- packages/remix-react/components.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index e013d6355a1..a69c9c7243b 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -770,7 +770,9 @@ export function processMeta( routeMeta: HtmlMetaDescriptor | HtmlMetaDescriptor[] ) { let items: HtmlMetaDescriptor[] = Array.isArray(routeMeta) - ? routeMeta + ? routeMeta.map((item) => + item.title ? { key: "title", content: item.title } : item + ) : Object.entries(routeMeta) .map(([key, value]) => { if (!value) return []; From 8913d13908f1127f5c930f0bdbde0cc0617c4288 Mon Sep 17 00:00:00 2001 From: kiliman Date: Wed, 6 Apr 2022 11:29:23 -0400 Subject: [PATCH 14/17] fix: Update tests --- integration/meta-test.ts | 227 +++++++++++---------------------------- 1 file changed, 62 insertions(+), 165 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index 0583ee2ae49..a587ffbdae2 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -18,7 +18,7 @@ test.describe("meta", () => { files: { "app/root.jsx": js` import { json } from "@remix-run/node"; - import { Meta, Links, Outlet, Scripts } from "@remix-run/react"; + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; export const loader = async () => json({ @@ -31,10 +31,6 @@ test.describe("meta", () => { description: data.description, "og:image": "https://picsum.photos/200/200", "og:type": data.contentType, // undefined - refresh: { - httpEquiv: "refresh", - content: "3;url=https://www.mozilla.org", - }, title: data.title, }); @@ -128,125 +124,8 @@ describe("meta array syntax", () => { fixture = await createFixture({ files: { "app/root.jsx": js` - import { json, Meta, Links, Outlet, Scripts } from "remix"; - -// export const loader = async () => -// json({ -// description: "This is a meta page", -// title: "Meta Page", -// }); - -// export const meta = ({ data }) => ({ -// charset: "utf-8", -// description: data.description, -// "og:image": "https://picsum.photos/200/200", -// "og:type": data.contentType, // undefined -// refresh: { -// httpEquiv: "refresh", -// content: "3;url=https://www.mozilla.org", -// }, -// title: data.title, -// }); - -// export default function Root() { -// return ( -// -// -// -// -// -// -// -// -// -// -// ); -// } -// `, - -// "app/routes/index.jsx": js` -// export default function Index() { -// return
This is the index file
; -// } -// `, -// }, -// }); - -// app = await createAppFixture(fixture); -// }); - -// afterAll(async () => await app.close()); - -// test("empty meta does not render a tag", async () => { -// let enableJavaScript = await app.disableJavaScript(); - -// await app.goto("/"); - -// await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( -// 'No element matches selector "meta[property="og:type"]"' -// ); -// await enableJavaScript(); -// }); - -// test("meta { charset } adds a ", async () => { -// let enableJavaScript = await app.disableJavaScript(); - -// await app.goto("/"); - -// expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); -// await enableJavaScript(); -// }); - -// test("meta { title } adds a ", async () => { -// let enableJavaScript = await app.disableJavaScript(); - -// await app.goto("/"); - -// expect(await app.getHtml("title")).toBeTruthy(); -// await enableJavaScript(); -// }); - -// test("meta { 'og:*' } adds a <meta property='og:*' />", async () => { -// let enableJavaScript = await app.disableJavaScript(); - -// await app.goto("/"); - -// expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); -// await enableJavaScript(); -// }); - -// test("meta { description } adds a <meta name='description' />", async () => { -// let enableJavaScript = await app.disableJavaScript(); - -// await app.goto("/"); - -// expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); -// await enableJavaScript(); -// }); - -// test("meta { refresh } adds a <meta http-equiv='refresh' content='3;url=https://www.mozilla.org' />", async () => { -// let enableJavaScript = await app.disableJavaScript(); - -// await app.goto("/"); - -// expect( -// await app.getHtml( -// 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' -// ) -// ).toBeTruthy(); -// await enableJavaScript(); -// }); -// }); - -describe("meta array-syntax", () => { - let fixture: Fixture; - let app: AppFixture; - - beforeAll(async () => { - fixture = await createFixture({ - files: { - "app/root.jsx": js` - import { json, Meta, Links, Outlet, Scripts } from "remix"; + import { json } from "@remix-run/node"; + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; export const loader = async () => json({ @@ -259,22 +138,32 @@ describe("meta array-syntax", () => { { name: "description", content: data.description }, { property: "og:image", content: "https://picsum.photos/200/200" }, { property: "og:type", content: data.contentType }, // undefined - { key: "http-equiv:refresh", httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, - { key: "title", content: data.title }, + { key: "httpEquiv:refresh", httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, + { title: data.title }, + { name: "viewport", content: "width=device-width, initial-scale=1" }, ]; export default function Root() { return ( <html lang="en"> - <head> - <Meta /> - </head> + <head> + <Meta /> + <Links /> + </head> + <body> + <Outlet /> + <Scripts /> + </body> </html> ); } `, "app/routes/index.jsx": js` + export const meta = ({ data }) => [ + { title: 'Override Title' }, + ]; + export default function Index() { return <div>This is the index file</div>; } @@ -291,60 +180,68 @@ describe("meta array-syntax", () => { let enableJavaScript = await app.disableJavaScript(); await app.goto("/"); - let head = await app.getHtml("head"); - console.log(head); + await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( 'No element matches selector "meta[property="og:type"]"' ); await enableJavaScript(); }); - // test("meta { charset } adds a <meta charset='utf-8' />", async () => { - // let enableJavaScript = await app.disableJavaScript(); + test("meta { charset } adds a <meta charset='utf-8' />", async () => { + let enableJavaScript = await app.disableJavaScript(); + + await app.goto("/"); + + expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); + await enableJavaScript(); + }); + + test("meta { title } adds a <title />", async () => { + let enableJavaScript = await app.disableJavaScript(); - // await app.goto("/"); + await app.goto("/"); - // expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); - // await enableJavaScript(); - // }); + expect(await app.getHtml("title")).toBeTruthy(); + await enableJavaScript(); + }); - // test("meta { title } adds a <title />", async () => { - // let enableJavaScript = await app.disableJavaScript(); + test("meta { title } overrides a <title />", async () => { + let enableJavaScript = await app.disableJavaScript(); - // await app.goto("/"); + await app.goto("/"); - // expect(await app.getHtml("title")).toBeTruthy(); - // await enableJavaScript(); - // }); + expect(await app.getHtml("title")).toBe("<title>Override Title"); + await enableJavaScript(); + }); - // test("meta { 'og:*' } adds a ", async () => { - // let enableJavaScript = await app.disableJavaScript(); + test("meta { 'og:*' } adds a ", async () => { + let enableJavaScript = await app.disableJavaScript(); - // await app.goto("/"); + await app.goto("/"); - // expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); - // await enableJavaScript(); - // }); + expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); + await enableJavaScript(); + }); - // test("meta { description } adds a ", async () => { - // let enableJavaScript = await app.disableJavaScript(); + test("meta { description } adds a ", async () => { + let enableJavaScript = await app.disableJavaScript(); - // await app.goto("/"); + await app.goto("/"); - // expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); - // await enableJavaScript(); - // }); + expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); + await enableJavaScript(); + }); - // test("meta { refresh } adds a ", async () => { - // let enableJavaScript = await app.disableJavaScript(); + test("meta { refresh } adds a ", async () => { + let enableJavaScript = await app.disableJavaScript(); - // await app.goto("/"); + await app.goto("/"); - // expect( - // await app.getHtml( - // 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' - // ) - // ).toBeTruthy(); - // await enableJavaScript(); - // }); + expect( + await app.getHtml( + 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' + ) + ).toBeTruthy(); + await enableJavaScript(); + }); }); From dfc3bcae34a7c5b57d9842a2ed69f925691a19d1 Mon Sep 17 00:00:00 2001 From: kiliman Date: Thu, 22 Sep 2022 14:18:08 -0500 Subject: [PATCH 15/17] fix: rebase on dev --- integration/meta-test.ts | 75 +++++++++++--------- packages/remix-react/__tests__/meta-test.tsx | 6 +- packages/remix-react/components.tsx | 1 + 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index a587ffbdae2..55157d3de8d 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -3,6 +3,7 @@ import { test, expect } from "@playwright/test"; import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; import type { Fixture, AppFixture } from "./helpers/create-fixture"; import { PlaywrightFixture } from "./helpers/playwright-fixture"; +import { compareDocumentPosition } from "domutils"; test.describe("meta", () => { let fixture: Fixture; @@ -31,6 +32,10 @@ test.describe("meta", () => { description: data.description, "og:image": "https://picsum.photos/200/200", "og:type": data.contentType, // undefined + refresh: { + httpEquiv: "refresh", + content: "3;url=https://www.mozilla.org", + }, title: data.title, }); @@ -116,11 +121,16 @@ test.describe("meta", () => { }); }); -describe("meta array syntax", () => { +test.describe("meta array syntax", () => { let fixture: Fixture; - let app: AppFixture; + let appFixture: AppFixture; + + // disable JS for all tests in this file + // to only disable them for some, add another test.describe() + // and move the following line there + test.use({ javaScriptEnabled: false }); - beforeAll(async () => { + test.beforeAll(async () => { fixture = await createFixture({ files: { "app/root.jsx": js` @@ -171,70 +181,61 @@ describe("meta array syntax", () => { }, }); - app = await createAppFixture(fixture); + appFixture = await createAppFixture(fixture); }); - afterAll(async () => await app.close()); - - test("empty meta does not render a tag", async () => { - let enableJavaScript = await app.disableJavaScript(); + test.afterAll(() => appFixture.close()); + test("empty meta does not render a tag", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); await expect(app.getHtml('meta[property="og:type"]')).rejects.toThrowError( 'No element matches selector "meta[property="og:type"]"' ); - await enableJavaScript(); }); - test("meta { charset } adds a ", async () => { - let enableJavaScript = await app.disableJavaScript(); - + test("meta { charset } adds a ", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); expect(await app.getHtml('meta[charset="utf-8"]')).toBeTruthy(); - await enableJavaScript(); }); - test("meta { title } adds a ", async () => { - let enableJavaScript = await app.disableJavaScript(); - + test("meta { title } adds a <title />", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); expect(await app.getHtml("title")).toBeTruthy(); - await enableJavaScript(); }); - test("meta { title } overrides a <title />", async () => { - let enableJavaScript = await app.disableJavaScript(); - + test("meta { title } overrides a <title />", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); expect(await app.getHtml("title")).toBe("<title>Override Title"); - await enableJavaScript(); }); - test("meta { 'og:*' } adds a ", async () => { - let enableJavaScript = await app.disableJavaScript(); - + test("meta { 'og:*' } adds a ", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); expect(await app.getHtml('meta[property="og:image"]')).toBeTruthy(); - await enableJavaScript(); }); - test("meta { description } adds a ", async () => { - let enableJavaScript = await app.disableJavaScript(); - + test("meta { description } adds a ", async ({ + page, + }) => { + let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); expect(await app.getHtml('meta[name="description"]')).toBeTruthy(); - await enableJavaScript(); }); - test("meta { refresh } adds a ", async () => { - let enableJavaScript = await app.disableJavaScript(); - + test("meta { refresh } adds a ", async ({ + page, + }) => { + let app = new PlaywrightFixture(appFixture, page); await app.goto("/"); expect( @@ -242,6 +243,14 @@ describe("meta array syntax", () => { 'meta[http-equiv="refresh"][content="3;url=https://www.mozilla.org"]' ) ).toBeTruthy(); - await enableJavaScript(); + }); + + test("meta supports multiple items with same name/property />", async ({ + page, + }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + + expect(await app.getElement('meta[property="og:image"]')).toHaveLength(3); }); }); diff --git a/packages/remix-react/__tests__/meta-test.tsx b/packages/remix-react/__tests__/meta-test.tsx index 7c773540000..c540ba507d9 100644 --- a/packages/remix-react/__tests__/meta-test.tsx +++ b/packages/remix-react/__tests__/meta-test.tsx @@ -46,9 +46,9 @@ describe("meta", () => { // console.log(html); // title should override the title from the first meta function - expect(map.get("title").content).toBe("Updated title"); + expect(map.get("title")!.content).toBe("Updated title"); // viewport should be added - expect(map.get("viewport").content).toBe( + expect(map.get("viewport")!.content).toBe( "width=device-width, initial-scale=1" ); }); @@ -63,11 +63,11 @@ function getMeta(data: any, metaFunctions: MetaFunction[]) { data, parentsData: {}, params: {}, + // @ts-expect-error location: null, }); if (routeMeta) { processMeta(meta, routeMeta); - //console.log(meta, routeMeta); } }); diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index a69c9c7243b..4913c6763b0 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -56,6 +56,7 @@ import type { Fetcher, Submission, } from "./transition"; +import { keyBy } from "lodash"; //////////////////////////////////////////////////////////////////////////////// // RemixEntry From 6d5eb0d02de1122273500c52e73fe8be70fde6c9 Mon Sep 17 00:00:00 2001 From: kiliman Date: Thu, 22 Sep 2022 14:19:05 -0500 Subject: [PATCH 16/17] feat: add support for multiple entries with same property name --- integration/meta-test.ts | 6 +++ packages/remix-react/components.tsx | 84 +++++++++++++---------------- 2 files changed, 42 insertions(+), 48 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index 55157d3de8d..2d12e148f09 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -141,12 +141,18 @@ test.describe("meta array syntax", () => { json({ description: "This is a meta page", title: "Meta Page", + contentType: undefined, }); export const meta = ({ data }) => [ { key: "charset", content: "utf-8" }, { name: "description", content: data.description }, + { property: "og:image", content: "https://picsum.photos/300/300" }, + { property: "og:image:width", content: "300" }, + { property: "og:image:height", content: "300" }, { property: "og:image", content: "https://picsum.photos/200/200" }, + { property: "og:image", content: "https://picsum.photos/500/1000" }, + { property: "og:image:height", content: "1000" }, { property: "og:type", content: data.contentType }, // undefined { key: "httpEquiv:refresh", httpEquiv: "refresh", content: "3;url=https://www.mozilla.org" }, { title: data.title }, diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 4913c6763b0..4792cc097fa 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -725,38 +725,21 @@ export function Meta() { return ( <> - {[...meta.entries()].map(([key, value]) => { - if (key === "title" && typeof value.content === "string") { - return {value.content}; + {Array.from(meta.entries()).map(([key, descriptor]) => { + let { name, property, content } = descriptor; + if (key === "title" && typeof content === "string") { + return {content}; } - if (key === "charset" && typeof value.content === "string") { - return ; + if (key === "charset" && typeof content === "string") { + return ; } - - if (name === "title") { - return {String(value)}; + if (property !== undefined) { + return ; } - - // Open Graph tags use the `property` attribute, while other meta tags - // use `name`. See https://ogp.me/ - let isOpenGraphTag = name.startsWith("og:"); - return [value].flat().map((content) => { - if (isOpenGraphTag) { - return ( - - ); - } - - if (typeof content === "string") { - return ; - } - - return ; - }); + if (name !== undefined) { + return ; + } + return ; })} ); @@ -770,9 +753,12 @@ export function processMeta( meta: MetaMap, routeMeta: HtmlMetaDescriptor | HtmlMetaDescriptor[] ) { + // normalize routeMeta to array format let items: HtmlMetaDescriptor[] = Array.isArray(routeMeta) ? routeMeta.map((item) => - item.title ? { key: "title", content: item.title } : item + item.title + ? { key: "title", content: item.title } + : { key: generateKey(item), ...item } ) : Object.entries(routeMeta) .map(([key, value]) => { @@ -789,14 +775,24 @@ export function processMeta( [propertyName]: key, content, })) - : { key, [propertyName]: key, content: value }; + : typeof value === "object" + ? { + key: `${key}.${(value as Record)?.content}`, + ...(value as Record), + } + : { + key: propertyName === "name" ? key : `${key}.${value}`, + [propertyName]: key, + content: assertString(value), + }; }) .flat(); - items.forEach((item) => { - let [key, value] = computeKey(item); - // child routes always override parent routes - meta.set(key, value); + // add items to the meta map by key (only add if content is defined) + items.forEach(({ key, ...rest }) => { + if (key && rest && rest.content) { + meta.set(key, rest); + } }); } @@ -808,22 +804,14 @@ function assertString(value: string | string[]): string { } function generateKey(item: HtmlMetaDescriptor) { - console.warn("No key provided to meta", item); + if (item.key) return item.key; + if (item.title) return "title"; + if (item.charset || item.charSet) return "charset"; + if (item.name) return item.name; + if (item.property) return `${item.property}.${item.content}`; return JSON.stringify(item); // generate a reasonable key } -function computeKey(item: HtmlMetaDescriptor): [string, HtmlMetaDescriptor] { - let { - name, - property, - title, - key = name ?? property ?? title ?? generateKey(item), - ...rest - } = item; - - return [key, { name, property, title, ...rest }]; -} - /** * Tracks whether Remix has finished hydrating or not, so scripts can be skipped * during client-side updates. From 87594af3f1f6f0ef06c1a712b67f5ef1657b5137 Mon Sep 17 00:00:00 2001 From: kiliman Date: Thu, 22 Sep 2022 14:23:41 -0500 Subject: [PATCH 17/17] fix: fix lint errors --- integration/meta-test.ts | 1 - packages/remix-react/components.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/integration/meta-test.ts b/integration/meta-test.ts index 2d12e148f09..4ba39a65fea 100644 --- a/integration/meta-test.ts +++ b/integration/meta-test.ts @@ -3,7 +3,6 @@ import { test, expect } from "@playwright/test"; import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; import type { Fixture, AppFixture } from "./helpers/create-fixture"; import { PlaywrightFixture } from "./helpers/playwright-fixture"; -import { compareDocumentPosition } from "domutils"; test.describe("meta", () => { let fixture: Fixture; diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 4792cc097fa..16eb3315e4e 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -56,7 +56,6 @@ import type { Fetcher, Submission, } from "./transition"; -import { keyBy } from "lodash"; //////////////////////////////////////////////////////////////////////////////// // RemixEntry