-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add helmet to web for meta tags | Prerender support #2909
Conversation
44363c0
to
692a1c8
Compare
packages/web/src/index.ts
Outdated
export { Helmet as Head, Helmet } from 'react-helmet-async' | ||
export { HelmetProvider } from 'react-helmet-async' |
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.
Are you exporting Head, Helmet and HelmetProvider here because other people might want to use those instead of Head
? My suggestion would be to just export Head
and abstract away Helmet.
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.
Yeah - my idea was to officially document <Head>
but given the popularity of Helmet, I thought maybe users might prefer to use that name instead. Doesn’t hurt right?
Can remove HelmetProvider though, not really sure what I was thinking there.
@@ -7,6 +9,7 @@ declare global { | |||
* This global is set to true by the prerendering CLI command. | |||
*/ | |||
__REDWOOD__PRERENDERING: boolean | |||
__REDWOOD__HELMET_CONTEXT: { helmet?: HelmetData } |
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.
Is this for pre-rendering?
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.
Yup - helmet async gathers up all the headers in this variable during prerender - will also be the same if we do this in builder functions in the future.
Co-authored-by: Peter Pistorius <[email protected]>
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.
Will you be keeping RedwoodProvider
?
Yes, that's the plan. |
Ah, sorry, I got
|
…edwood into feature/prerender-helmet-support * 'feature/prerender-helmet-support' of github.com:dac09/redwood: upgrade Prisma v2.26.0 (redwoodjs#2915) Upgrade supabase-js with Discord auth support (redwoodjs#2965) resolve storybook config to Framework (redwoodjs#2892) Update packages/web/package.json
LGTM! |
… fix/cli-command-names * 'fix/cli-command-names' of github.com:dac09/redwood: Add helmet to web for meta tags | Prerender support (redwoodjs#2909)
…endabot-updates-070621 * 'main' of github.com:redwoodjs/redwood: Add helmet to web for meta tags | Prerender support (#2909)
What?
Adds support for using react-helmet-async to modify , including open graph metadata, during prerender (and at run time too)
<Head>
or<Helmet>
components (aliased)<MetaTags>
component that lets you set commonly used meta tags for opengraph SEO/UnfurlingNotes
Linked to #2001
For release notes 👇
We need to make sure the
RedwoodProvider
wraps your entire App. In App.tsx:This is an optional change, as your Redwood app will continue to work without this, but we recommend making the change now, so you're up-to-date with the templates we use in our tests.