-
Notifications
You must be signed in to change notification settings - Fork 9
Structured data for article #6052
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
Changes from all commits
6f7f2ba
2f5b4a7
512fd43
aad12af
9793233
345a882
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,27 @@ | ||
| export const dynamic = "error"; | ||
|
|
||
| import { JsonLd } from "@dextinity/site-nextjs"; | ||
| import type { VisibilityParam } from "@src/middleware/domainRewrite"; | ||
| import { NewsPage } from "@src/news/NewsPage"; | ||
| import { fetchNewsList } from "@src/news/NewsPage.loader"; | ||
| import { setVisibilityParam } from "@src/util/ServerContext"; | ||
| import { buildNewsItemList } from "@src/util/structuredData/buildNewsItemList"; | ||
| import type { ItemList } from "schema-dts"; | ||
|
|
||
| export default async function NewsIndexPage({ params }: PageProps<"/[visibility]/[domain]/[language]/news">) { | ||
| const { visibility, domain, language } = await params; | ||
| setVisibilityParam(visibility as VisibilityParam); | ||
| return <NewsPage scope={{ domain, language }} initialData={await fetchNewsList({ scope: { domain, language }, limit: 2 })} />; | ||
|
|
||
| const scope = { domain, language }; | ||
| const initialData = await fetchNewsList({ scope, limit: 2 }); | ||
|
|
||
| // Only the initially rendered page is encoded — client-side "Load more" items are not part of the ItemList. | ||
| const itemList = buildNewsItemList({ items: initialData.nodes, scope }); | ||
|
|
||
| return ( | ||
| <> | ||
| <JsonLd<ItemList> data={itemList} /> | ||
| <NewsPage scope={scope} initialData={initialData} /> | ||
| </> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,12 @@ | ||
| import { JsonLd } from "@dextinity/site-nextjs"; | ||
| import type { PublicSiteConfig } from "@src/site-configs"; | ||
| import type { Organization, WithContext } from "schema-dts"; | ||
| import { buildOrganization } from "@src/util/structuredData/buildOrganization"; | ||
| import type { Organization } from "schema-dts"; | ||
|
|
||
| interface Props { | ||
| siteConfig: PublicSiteConfig; | ||
| } | ||
|
|
||
| function toAbsoluteUrl(url: string, siteUrl: string): string { | ||
| return new URL(url, siteUrl).toString(); | ||
| } | ||
|
|
||
| export function OrganizationJsonLd({ siteConfig }: Props) { | ||
| const { organization, url: siteUrl } = siteConfig; | ||
|
|
||
| const data: WithContext<Organization> = { | ||
| "@context": "https://schema.org", | ||
| "@type": "Organization", | ||
| name: organization.name, | ||
| url: organization.url ?? siteUrl, | ||
| ...(organization.logo ? { logo: toAbsoluteUrl(organization.logo, siteUrl) } : {}), | ||
| ...(organization.sameAs?.length ? { sameAs: organization.sameAs } : {}), | ||
| ...(organization.description ? { description: organization.description } : {}), | ||
| }; | ||
|
|
||
| return <JsonLd<Organization> data={data} />; | ||
| return <JsonLd<Organization> data={buildOrganization(siteConfig)} />; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import type { PublicSiteConfig } from "@src/site-configs"; | ||
|
|
||
| let siteConfigs: PublicSiteConfig[]; | ||
|
|
||
| export function getSiteConfigs() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please keep changes in a PR to a minimum and move this refactor (move of getSiteConfigs/getSiteConfigForDomain to it's own file) to an individual PR
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this refactor makes sense. Here's the rationale from the PR description:
But |
||
| if (!siteConfigs) { | ||
| const json = process.env.PUBLIC_SITE_CONFIGS; | ||
| if (!json) { | ||
| throw new Error("process.env.PUBLIC_SITE_CONFIGS must be set."); | ||
| } | ||
| siteConfigs = JSON.parse(atob(json)) as PublicSiteConfig[]; | ||
| } | ||
| return siteConfigs; | ||
| } | ||
|
|
||
| export function getSiteConfigForDomain(domain: string) { | ||
| const siteConfig = getSiteConfigs().find((siteConfig) => siteConfig.scope.domain === domain); | ||
| if (!siteConfig) { | ||
| throw new Error(`SiteConfig not found for domain ${domain}`); | ||
| } | ||
| return siteConfig; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import type { DamImageBlockData } from "@src/blocks.generated"; | ||
| import type { ContentScope } from "@src/site-configs"; | ||
| import { createSitePath } from "@src/util/createSitePath"; | ||
| import { getSiteConfigForDomain } from "@src/util/getSiteConfigs"; | ||
| import type { Article, WithContext } from "schema-dts"; | ||
|
|
||
| import { buildOrganizationNode } from "./buildOrganization"; | ||
| import { damImageToAbsoluteUrl } from "./damImageToAbsoluteUrl"; | ||
|
|
||
| type BuildArticleOptions = { | ||
| news: { | ||
| title: string; | ||
| image: DamImageBlockData; | ||
| date: string; | ||
| updatedAt: string; | ||
| slug: string; | ||
| }; | ||
|
Comment on lines
+11
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice if we could pick the types from |
||
| scope: ContentScope; | ||
| }; | ||
|
|
||
| export function buildArticle({ news, scope }: BuildArticleOptions): WithContext<Article> { | ||
| const siteConfig = getSiteConfigForDomain(scope.domain); | ||
| const organization = buildOrganizationNode(siteConfig); | ||
| const image = damImageToAbsoluteUrl(news.image, siteConfig.url); | ||
| const detailUrl = `${siteConfig.url}${createSitePath({ scope: { language: scope.language }, path: `/news/${news.slug}` })}`; | ||
|
|
||
| return { | ||
| "@context": "https://schema.org", | ||
| "@type": "Article", | ||
| headline: news.title, | ||
| ...(image ? { image } : {}), | ||
| datePublished: news.date, | ||
| dateModified: news.updatedAt, | ||
| author: organization, | ||
| publisher: organization, | ||
|
Comment on lines
+34
to
+35
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imho we should not include the same organisation object, that we already include on every page, here as author and publisher. I'd use auther/publisher ONLY if we would have real values (and we would store the author per news - and display them in the site) |
||
| mainEntityOfPage: detailUrl, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import type { ContentScope } from "@src/site-configs"; | ||
| import { createSitePath } from "@src/util/createSitePath"; | ||
| import { getSiteConfigForDomain } from "@src/util/getSiteConfigs"; | ||
| import type { ItemList, WithContext } from "schema-dts"; | ||
|
|
||
| type NewsItemListEntry = { | ||
| title: string; | ||
| slug: string; | ||
| scope: { language: string }; | ||
| }; | ||
|
|
||
| type BuildNewsItemListOptions = { | ||
| items: NewsItemListEntry[]; | ||
| scope: ContentScope; | ||
| }; | ||
|
|
||
| export function buildNewsItemList({ items, scope }: BuildNewsItemListOptions): WithContext<ItemList> { | ||
| const siteUrl = getSiteConfigForDomain(scope.domain).url; | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
|
|
||
| return { | ||
| "@context": "https://schema.org", | ||
| "@type": "ItemList", | ||
| itemListElement: items.map((item, index) => ({ | ||
| "@type": "ListItem", | ||
| position: index + 1, | ||
| name: item.title, | ||
| url: `${siteUrl}${createSitePath({ scope: { language: item.scope.language }, path: `/news/${item.slug}` })}`, | ||
| })), | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import type { PublicSiteConfig } from "@src/site-configs"; | ||
| import type { Organization, WithContext } from "schema-dts"; | ||
|
|
||
| // schema-dts types `Organization` as a union that includes `string`; the builders only ever produce the object form. | ||
| type OrganizationNode = Exclude<Organization, string>; | ||
|
|
||
| function toAbsoluteUrl(url: string, siteUrl: string): string { | ||
| return new URL(url, siteUrl).toString(); | ||
| } | ||
|
|
||
| export function buildOrganizationNode(siteConfig: PublicSiteConfig): OrganizationNode { | ||
| const { organization, url: siteUrl } = siteConfig; | ||
|
|
||
| return { | ||
| "@type": "Organization", | ||
| name: organization.name, | ||
| url: organization.url ?? siteUrl, | ||
| ...(organization.logo ? { logo: toAbsoluteUrl(organization.logo, siteUrl) } : {}), | ||
| ...(organization.sameAs?.length ? { sameAs: organization.sameAs } : {}), | ||
| ...(organization.description ? { description: organization.description } : {}), | ||
| }; | ||
| } | ||
|
|
||
| export function buildOrganization(siteConfig: PublicSiteConfig): WithContext<Organization> { | ||
| return { "@context": "https://schema.org", ...buildOrganizationNode(siteConfig) }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { generateImageUrl } from "@dextinity/site-nextjs"; | ||
| import type { DamImageBlockData } from "@src/blocks.generated"; | ||
|
|
||
| function damImageToUrl(image: DamImageBlockData): string | undefined { | ||
| const props = image.block?.props; | ||
|
|
||
| if (!props) { | ||
| return undefined; | ||
| } | ||
|
|
||
| if ("urlTemplate" in props && props.damFile?.image) { | ||
| const { width, height } = props.damFile.image; | ||
| return generateImageUrl({ src: props.urlTemplate, width }, width / height); | ||
| } | ||
|
Comment on lines
+11
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This generates an image in its original size and aspect ratio. Google recommends to generate multiple images:
|
||
|
|
||
| return props.damFile?.fileUrl; | ||
| } | ||
|
|
||
| export function damImageToAbsoluteUrl(image: DamImageBlockData, siteUrl: string): string | undefined { | ||
| const url = damImageToUrl(image); | ||
|
|
||
| return url ? new URL(url, siteUrl).toString() : undefined; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it was required in the ticket, but I'm questioning this.
I don't think we should use an ItemList JsonLd here when we have a detail page that has a full Article JsonLd