Skip to content
This repository has been archived by the owner on Dec 31, 2022. It is now read-only.

bug: NextBannerMeta references a incorrect og:image in various situations #82

Open
jariz opened this issue Jun 4, 2022 · 3 comments
Open

Comments

@jariz
Copy link

jariz commented Jun 4, 2022

I've ran into various use cases where the og:image does not reference the actual path in the public folder.

It...

  • Doesn't take the locale into account, e.g. when I'm on /docs with a locale set to en, it will generate the following: <meta property="og:image" content="contoso.com/next-banner-generated/docs..."/> instead of <meta property="og:image" content="contoso.com/next-banner-generated/en/docs..."/> which is where the generated images get placed.
    So it should either
    • Place the images at the root, /docs.jpg instead of /en/docs.jpg in this case.
    • Include the locale.
  • Doesn't strip the trailing slash (next.config trailingSlash), aka it will render
    <meta property="og:image" content="contoso.com/next-banner-generated/docs/.jpg"/>
    while this should be content="contoso.com/next-banner-generated/docs.jpg"
@jariz jariz changed the title bug: NextBannerMeta references a og:image in various situations bug: NextBannerMeta references a incorrect og:image in various situations Jun 4, 2022
@jariz
Copy link
Author

jariz commented Jun 5, 2022

I made a replacement version for my own needs here:
https://github.com/plutoniummod/landing/blob/develop/src/components/NextBannerMeta.jsx

If i have time I'll open a PR but I'm not sure if it satisfies everyone's needs.

@alvarlagerlof
Copy link
Owner

alvarlagerlof commented Jun 5, 2022

Adding support for locale seems like a great idea, I suppose adding the locale always is better in case the images should have different langs too. But the example code does not work for index cases. For my example folder, the / route generated next-banner-alvarlagerlof.vercel.app/next-banner-generated.jpg.

The explicit / to index is needed to not have folder .jpg. Maybe this instead?

let url = `${domain}/${outputDir}`;

  if (locale) {
    url += `/${locale}`;
  }

  url += asPath;

  // remove #anchors
  url = url.replace(/#[a-zA-Z-]*/, "");

  // add "index" to if on root page
  if (asPath === "/") {
    url.slice(0, -1);
    url += "index";
  }

  url += ".jpg";

@alvarlagerlof
Copy link
Owner

It also looks like domain routing might make including the locale in the URL outright, problematic.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants