Skip to content
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

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

Open
1 task done
joshuabaker opened this issue Oct 24, 2023 · 20 comments
Open
1 task done

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

joshuabaker opened this issue Oct 24, 2023 · 20 comments
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@joshuabaker
Copy link
Contributor

joshuabaker commented Oct 24, 2023

Link to the code that reproduces this issue

#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

  • I verified that the issue exists in the latest Next.js 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

@joshuabaker joshuabaker added the bug Issue was opened via the bug report template. label Oct 24, 2023
@github-actions github-actions bot added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Oct 24, 2023
@gijsbotje
Copy link

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
Copy link

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. 😉

@joshuabaker
Copy link
Contributor Author

@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
Copy link

sanneh2 commented Jan 6, 2024

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
Copy link

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

@lionelrudaz
Copy link

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
Copy link

sanneh2 commented Jan 22, 2024

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
Copy link
Contributor Author

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
Copy link

ceIia commented Jan 23, 2024

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
Copy link

fsa317 commented Jan 24, 2024

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

@lionelrudaz
Copy link

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
Copy link

d-reader-josip commented Apr 30, 2024

Anyone found any fixes/workarounds perhaps?

@polesapart
Copy link

polesapart commented May 27, 2024

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

@joshuabaker
Copy link
Contributor Author

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
Copy link
Member

huozhi commented May 29, 2024

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:

  • single route: /opengraph-image
  • multi routes with generateImageMetadata(): /opengraph-image/[id]

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
Copy link
Contributor Author

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

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

@huozhi
Copy link
Member

huozhi commented May 29, 2024

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

samcx pushed a commit that referenced this issue May 29, 2024
### What

Removing the `/[...catchall]/<metadata image convention>.js` case from
docs

### Why

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: 

- single route: `/opengraph-image`
- multi routes with `generateImageMetadata()`:  `/opengraph-image/[id]`

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.

Related #48106 
Related #57349
@vpb11
Copy link

vpb11 commented Jul 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

No branches or pull requests

10 participants