search

redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.27k stars 991 forks source link

RFC: Improving SEO with React Helmet 🪖 #2001

Closed simoncrypta closed 3 years ago

simoncrypta commented 3 years ago

The goal is to find how to provide a better SEO out of the box

This continues the discussions from #1847 for the internalization and also #217 for the conversation about SEO.

Let's start with meta tags that should be there by default in a Redwood app

I take inspiration from "The Ultimate Guide to SEO Meta Tags" blog post

To have with every circumstance :

To have with specific circumstances :

You can see we have most of the meta tags we need for SEO, but none is dynamic. This means that most of these tags are can be irrelevant.

How to make good dynamic meta tags

Proposition 1 :

Simply add React Helmet to generate page template with these tags 

    <Helmet>
      <title>${pascalName}Page</title>
    </Helmet>

keep the other default tags in the HTML file and write documentation for all the specific circumstances

Proposition 2 :

Create an internal component with all the meta tags like this Gatsby template who are imported into every page with props for dynamic tags and also have a config file for static tags.

We need your opinion and proposition

We want a better SEO out of the box for everyone, so if you think we should cover less or more meta tags by default or another way to make it dynamic by pages, I want to know!

cc @thedavidprice @clairefro

simoncrypta commented 3 years ago

To cover the discussion with @clairefro adding <html lang={i18n.language} /> inside Helmet would set the present language for html lang.

dthyresson commented 3 years ago

Simply add React Helmet to generate page template

I have been using React Helmet, but I have done all the "setup" in a layout and then my page -- or sometimes a control/cell -- if the title is data-driven -- overrides the layout's title (or other meta tags).

This is especially common when showing a blog page with an article title/author/date and then a social image that is the article's photo, etc.

simoncrypta commented 3 years ago

I also currently done the helmet setup in layout with props from pages. I didn't think about data-driven titles, is a good point @dthyresson

thedavidprice commented 3 years ago

Huge thanks for getting started on research and this proposal @simoncrypta! This is exactly what we needed. Also, I'm looping in @dac09; he's leading Prerender and has a lot of Jamstack + SEO experience.

Question for everyone: Is there any reason why we shouldn't have built-in SEO support from the start?

@dthyresson to your comment about using React Helmet within a layout, do you think that will need to change given the new <Set /> for Routes.js?

dac09 commented 3 years ago

Helmet is pretty good for this sort of thing, do you guys think we should include it inside redwood? I'm not sure having it change the title by default for every route is the way to go though, opt-in makes more sense to me

I've put up a draft PR here about using helmet for prerendered pages. https://github.com/redwoodjs/redwood/pull/2066/files

Also note, that I'm using react-helmet-async not helmet, because the API is slightly better for SSR

thedavidprice commented 3 years ago

To discuss:

simoncrypta commented 3 years ago

All right, so helmet is now included inside redwood #2909 ✅ thanks @dac09 . Do we wait for another PR to change the title by default for every route by including in page? I think something like this could be valuable to be added by default :

    <>
      <Helmet>
        <title>testPage </title>
        <meta name="description" content=" My default route is named test" />
      </Helmet>
      <h1>testPage</h1>
      <p>
        Find me in <code>./web/src/pages/testPage/testPage.tsx</code>
      </p>
      <p>
        My default route is named <code>test</code>, link to me with `
        <Link to={routes.test()}>Test</Link>`
      </p>
    </>

specific considerations for creating the "right" foundation to then add elements for i18n [...]

Also, for i18n, I'm currently testing some configurations to have the right foundation. I think it can wait to see where #2429 will be going for optional path parameters. For now, the best method I find is a React component with a useEffect who listen the i18n useTranslation hook and the path parameter for the language to change the translation and the HTML lang attribute with helmet.

So, @thedavidprice do you think we should close this issue or not?

thedavidprice commented 3 years ago

Not done yet!

There are definitely some great, possible next steps here:

  1. add defaults to Page generator
  2. change the title by default for every route (configurable to opt out)
  3. any/all default i18n support

@simoncrypta what do you think about helping out with any/all of these?

@dac09 I know you've talked about making SEO/OG dynamic. Is that something you're considering for v1?

simoncrypta commented 3 years ago

I will help with pleasure @thedavidprice, I will open a draft PR tomorrow for all of these.

When you said

any/all default i18n support

Do mean that when i18n is set, the page generator will also add the useTranslation hook and the helmet config for language ?

dac09 commented 3 years ago

I would love to carve 30 mins out to discuss this with you @simoncrypta - there's a lot of subtleties here that I don't think we can capture over issue chat :)

dac09 commented 3 years ago

@simoncrypta and I had a quick tete-a-tete, and we think we have a way forward with handling titles for page generators, and also making it easier to set the default title of the app through redwood.toml.

This gist:

dac09 commented 3 years ago

Closing this as implemented in #3026 and #2909