Skip to content

Conversation

@ChristopherChudzicki
Copy link
Contributor

What are the relevant tickets?

Followup to #1671

Description (What does it do?)

This PR moves metadadataBase from root page to root layout.

Child pages load all their parent layouts (but not parent pages!) so the earlier positioning of metadataBase was only affecting the home page.

How can this be tested?

  1. Despite what the docs say here, it seems like metadataBase does not work in local dev mode. So set NODE_ENV=production and NEXT_PUBLIC_ORIGIN=https://learn.mit.edu, then run yarn workspace main build yarn workspace main start

    • you should NOT see any warnings like

    ⚠ metadataBase property in metadata export is not set for resolving social open graph or twitter images, using "http://localhost:8062". See https://nextjs.org/docs/app/api-reference/functions/generate-metadata#metadatabase

  2. Visit your local production server and run $$('meta').forEach(e => console.log(e.outerHTML)) in the dev console

    • all pages, including homepage, should have og:image urls beginning with learn.mit.edu

@@ -1,3 +1,4 @@
"use client"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are all client components for now because they use runtime CSS-in-JS (Emotion) which depends on react useContext.

Marking them as use-client allows importing them into server components.

@@ -1,5 +1,3 @@
"use client"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

layout.tsx needs to be a server component in order to define metadata

searchParams,
title: `${channelDetails.title}`,
description: channelDetails.public_description,
image: channelDetails.configuration.logo,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated to the other changes, but...

  • we noticed the metadataBase issue because channel og:image were not working
  • og:image cannot be an SVG, so without this change, they still won't work.

@ChristopherChudzicki ChristopherChudzicki added the Needs Review An open Pull Request that is ready for review label Oct 10, 2024
@ChristopherChudzicki ChristopherChudzicki changed the title Cc/fix metadata base Make metadataBase apply to all pages, not just homepage Oct 10, 2024
@ChristopherChudzicki ChristopherChudzicki marked this pull request as ready for review October 10, 2024 17:12
Copy link
Contributor

@jonkafton jonkafton left a comment

Choose a reason for hiding this comment

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

Works!.. in prod mode yarn build; yarn start;

Interesting and odd that it does not work in dev mode. You mentioned that's apparently the case on Slack, but didn't see that info while scanning the docs.

@ChristopherChudzicki ChristopherChudzicki merged commit cad729a into nextjs Oct 10, 2024
12 checks passed
@odlbot odlbot mentioned this pull request Oct 22, 2024
74 tasks
@rhysyngsun rhysyngsun deleted the cc/fix-metadataBase branch February 7, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants