vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.49k stars 26.8k forks source link

`opengraph-image` and `twitter-image` breaks in dynamic routes #57349

Open joshuabaker opened 11 months ago

joshuabaker commented 11 months ago

Link to the code that reproduces this issue

https://github.com/vercel/next.js/issues/49630

To Reproduce

Use opengraph-image.tsx or twitter-image.tsx in any dynamic routes (i.e. /articles/[slug] and /[...slug]).

Current vs. Expected behavior

Next doesn’t currently consider these routes, but should.

Error: Catch-all must be the last part of the URL.

From a DX perspective, having these co-located with pages makes perfect sense, but the handling is broken due to ambiguous routing. It’s not clear if a request to /articles/twitter-image is to load the Twitter image for /articles or an article with the slug twitter-image. Next defaults to the latter and therefore doesn’t support image generation on dynamic paths.

The workarounds are all hacky and require us to replicate the entire app directory just to handle dynamic cases. That’s lots of boilerplate.

Possible Solution

Could these routes be switched to instead prefix the route with the handler trigger like the image transformer does (i.e. /_next/twitter-image/articles)? This way the handler can accept route params and not conflict with page routing.

Verify canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:43 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T6000
Binaries:
  Node: 18.18.2
  npm: 9.8.1
  Yarn: 1.22.19
  pnpm: 7.14.0
Relevant Packages:
  next: 13.5.7-canary.23
  eslint-config-next: 13.5.6
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 4.9.5
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

App Router, Metadata (metadata, generateMetadata, next/head), Routing (next/router, next/navigation, next/link)

Additional context

Related issues: #48106, #48162, #49630, #56963

gijsbotje commented 11 months ago

Looking through the docs, this section has an example with a catch-all route. I ran into the same issue with catch-all routes not able to generate the image with the error you mentioned.

Either the docs should mention a workaround or it should be supported.

lionelrudaz commented 9 months ago

Ran exactly on the same issue tonight. Trying to figure out what I did wrong, then going to the docs only to see that it's supposed to work.

CleanShot 2023-12-11 at 21 40 49@2x

https://github.com/vercel/next.js/issues/57403 is a similar issue.

I hope there will be a solution, as the lack of support on catch all routes kinda defeats the whole point.

It's working as a charm on dynamic [slug] routes though. 😉

joshuabaker commented 9 months ago

@huozhi Do you have any thoughts on this? Could the implementation switch away from the default route handling to the /_next path or similar, in order to support catch-all segments? Some sort of variation seems warranted to avoid the edge case collisions.

sanneh2 commented 9 months ago

Ran exactly on the same issue tonight. Trying to figure out what I did wrong, then going to the docs only to see that it's supposed to work.

CleanShot 2023-12-11 at 21 40 49@2x

57403 is a similar issue.

I hope there will be a solution, as the lack of support on catch all routes kinda defeats the whole point.

It's working as a charm on dynamic [slug] routes though. 😉

Yes, I also read the docs that It would work and I followed it only to get slapped with an error that breaks an entire application and halts the development process. I was not reading an early alpha stage documentation from some random software that is being peddled on the sidewalk, so I was frustrated to see this happen.

For now I would like to have some peace and remove this feature from the documentation until it is addressed.

lionelrudaz commented 8 months ago

Anyone upgraded to 14.1 yet? Does it solve the issue?

lionelrudaz commented 8 months ago

Follow up: have upgraded to 14.1, the issue is still the same.

How can we help to fix this? I've never contributed to a framework, but I'm glad if I can help.

sanneh2 commented 8 months ago

Follow up: have upgraded to 14.1, the issue is still the same.

How can we help to fix this? I've never contributed to a framework, but I'm glad if I can help.

I've contributed a feedback to the Next.js docs that they should remove it from the documentation. Myself and other people have lost a lot of time following it.

Instead I suggest generating the dynamic opengraph image using the API folder and the next/og ImageResponse

Inside the app/ folder, in create an og folder with route.tsx and follow the following principles to create your OG:

/* eslint-disable @next/next/no-img-element */
import { ImageResponse } from 'next/og'
import { NextRequest } from 'next/server'

// Route segment config
export const runtime = 'edge'

const siteUrl = 'https://www'

// Image generation
export async function GET(req: NextRequest) {
  // Image metadata
  const size = {
    width: 1200,
    height: 630,
  }

  const contentType = 'image/png'
  const url = new URL(req.url)
  const title = url.searchParams.get('title')
  const slug = url.searchParams.get('slug')

  // Font
  /*   const interSemiBold = fetch(new URL('./Inter-SemiBold.ttf', import.meta.url)).then((res) =>
    res.arrayBuffer()
  ) */

  return new ImageResponse(
    (
      // ImageResponse JSX element
      <div
        style={{
          display: 'flex',
          height: '100%',
          width: '100%',
          alignItems: 'center',
          justifyContent: 'center',
          letterSpacing: '-.02em',
          fontWeight: 700,
          backgroundColor: '#ffffff',
        }}
      >
        <img
          src={`https://www.com/blog/static/images/${slug}/post-image.jpg`}
          width="100%"
          height="100%"
          style={{
            position: 'absolute',
            opacity: '0.2',
          }}
          alt={title + ' ima'}
        />

        <div
          style={{
            left: 42,
            top: 42,
            position: 'absolute',
            display: 'flex',
            alignItems: 'center',
            borderRadius: '20px',
          }}
        >
          <img
            width="78"
            height="78"
            src=""
            alt=" "
          />
          <span
            style={{
              marginLeft: 8,
              fontSize: 28,
              position: 'relative',
              bottom: '10px',
            }}
          >
            s.Com
          </span>
          <span style={{ position: 'relative', top: '20px', left: '-320px' }}>
            {' '}
            Turn
          </span>
        </div>

        <div
          style={{
            left: 0,
            top: '30',
            position: 'relative',
            display: 'flex',
            alignItems: 'center',
            background: 'white',
            width: '73%',
            height: '70%',
            borderRadius: '20px',
            padding: '20px',
          }}
        >
          <img
            src={`https://w/${slug}/image.jpg`}
            width="255"
            height="255"
            style={{
              borderRadius: '10px',
            }}
            alt={title + ' Pom'}
          />

          <div
            style={{
              top: '-10px',
              display: 'flex',
              flexWrap: 'wrap',
              justifyContent: 'flex-start',
              padding: '0px 0px',
              margin: '0px 22px',
              fontSize: 40,
              width: 'auto',
              maxWidth: 550,
              textAlign: 'left',
              color: 'black',
              lineHeight: 1.4,
              borderRadius: '10px',
              backgroundColor: ' white',
            }}
          >
            {title}
          </div>
          <div
            style={{
              color: '#e6004d',
              fontSize: 40,
              marginTop: '20px',
              position: 'absolute',
              bottom: '85px',
              left: '70%',
              display: 'flex',
            }}
          >
            {' '}
            Read More{' '}
          </div>
        </div>
      </div>
    ),
    // ImageResponse options
    {
      // For convenience, we can re-use the exported opengraph-image
      // size config to also set the ImageResponse's width and height.
      ...size,
      /*       fonts: [
        {
          name: 'Inter',
          data: await interSemiBold,
          style: 'normal',
          weight: 400,
        },
      ], */
    }
  )
}

As you can see at the top the dynamic data is passed through URL Search Params. like api/og?title="My Title"&desc="My Desc" Which will return the generated OpenGraph image in Next's Image Response from the Edge

To consume the dynamic Open Graph image in dynamic routes you call the API route in the metadata object or generateMetadata function like so:

  openGraph: {
      ...
      images: [`${siteMetadata.siteUrl}/api/og?title=${post.title}&slug=${slug}`], // The dynamic params
     ...
    },
joshuabaker commented 8 months ago

This shouldn't be removed from the docs. The underlying generator needs fixing.

Whilst your solution works (and is what we and others have in production) it doesn't work in quite the same way (not as performant).

ceIia commented 8 months ago

This shouldn't be removed from the docs. The underlying generator needs fixing.

Whilst your solution works (and is what we and others have in production) it doesn't work in quite the same way (not as performant).

@sanneh2's approach still makes use of Edge rendering. although the actual feature needs fixing, i don't see how it's less performant?

fsa317 commented 8 months ago

Just hit the same issue, in my situation the alternative solution would be much less performant.

lionelrudaz commented 5 months ago

Next 14.2 is out, does it solve the issue for you folks? Should we go ahead with the workaround from @sanneh2 ?

d-reader-josip commented 5 months ago

Anyone found any fixes/workarounds perhaps?

polesapart commented 4 months ago

The docs are still wrong as of v15.0.0-rc.0. Or, if you prefer, it's still unimplemented :-)

joshuabaker commented 4 months ago

Anyone found any fixes/workarounds perhaps?

Write your own route (e.g. /meta-image/[...path]/route.ts ) and pass that via getMetadata for dynamic paths.

huozhi commented 4 months ago

Since og image and icon routes are using catch-all routes to handle the both single or multiple routes. For instance:

app/opengraph-image.js can possibly generate two cases:

We're not able to detect if there's generateImageMetadata defined in the file before mapping the routes, and decided to create the different routes to either /opengraph-image or /opengraph-image/[id]. That's why we're using a catch-all routes to handle them rn.

I'll remove the unsupported case from docs in #66355

joshuabaker commented 4 months ago

@huozhi Why not fix this by simply adjusting the generated path?

/_next/opengraph-image/[...path] would work, no?

huozhi commented 4 months ago

@huozhi Why not fix this by simply adjusting the generated path?

/_next/opengraph-image/[...path] would work, no?

That will not work it still has the same issue /_next/opengraph-image/<page paths>/<og image id>. <page paths> can be a catch-all path, and same as <og image id> could be either not a segment or a segment param [id].

Ideally it should match the convention that matches the fs routes, and we can deide if we make it a /opengraph-image or /opengraph-image/[id] before mapping the routes. That would require some code analyze before doing it which is not easy to do atm, or maybe another convention to make it easier.

joshuabaker commented 4 months ago

I trust that I'm missing something internal here, but wouldn't this not be the same as the fs path pattern just with a prefix?

If /blog/article/[slug] takes precedent over /blog/[...path] could /_next/opengraph-image/blog/article/[slug] not equally take precedent over /_next/opengraph-image/blog/[...path].

joshuabaker commented 4 months ago

I'm meaning that these would be rewritten at build time from where they are in the fs routes to the reserved path space to work around the conflict.

vpb11 commented 3 months ago

Am I going crazy here or could they really just not be arsed to fix this XD, surely this would work : https://pastebin.com/A8Tm6Qj1