search

vercel / next.js

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

`next/font` interferes with header links from middleware. #69000

Open zenzen-sol opened 2 months ago

zenzen-sol commented 2 months ago

Link to the code that reproduces this issue

https://github.com/zenzen-sol/next-reproduction-template/tree/sol/repro-001-next-15-broken

To Reproduce

  1. pnpm i && pnpm run dev
  2. Reload http://localhost:3000 in a browser.
  3. Check the response header links in dev tools.

Examples:

Current vs. Expected behavior

The next-intl library's middleware injects "alternate" links into the response header for SEO purposes. At the same time, next/font injects "preload" header links into the response header. Both sets of links should appear in the response header. This was working correctly in Next 14.

Since NextJS 14.3.0-canary.43 and 15.0.0-canary.0, the "alternate" links from next-intl are no longer present when next/font is used.

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.6.0: Fri Jul  5 17:53:24 PDT 2024; root:xnu-10063.141.1~2/RELEASE_ARM64_T6020
  Available memory (MB): 32768
  Available CPU cores: 12
Binaries:
  Node: 20.11.1
  npm: 10.2.4
  Yarn: 3.6.1
  pnpm: 9.7.1
Relevant Packages:
  next: 15.0.0-rc.0 // Latest available version is detected (15.0.0-rc.0).
  eslint-config-next: N/A
  react: 19.0.0-rc-19bd26be-20240815
  react-dom: 19.0.0-rc-19bd26be-20240815
  typescript: 5.5.4
Next.js Config:
  output: N/A

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

Font (next/font), Internationalization (i18n), Metadata, Middleware

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

next dev (local), Vercel (Deployed)

Additional context

I tested my reproduction against canary releases back to 14.3.0-canary.0. This issue first appears in v14.3.0-canary.43, and is not present in 14.3.0-canary.42.

If I had to guess, the issue is related to the changes made to packages/next/src/server/app-render/app-render.tsx in that prerelease.

samcx commented 2 months ago

@zenzen-sol This is likely because the headers size was limited, so next/font was taking precedence.

We expanded these headers →

I am taking a look to confirm!

samcx commented 2 months ago

@zenzen-sol :hmm:

Despite the headers size increase, I only see the next-intl's link headers, which probably makes sense (React calculates what is the best items to add to the link headers).

CleanShot 2024-08-21 at 16 56 44@2x

zenzen-sol commented 2 months ago

@samcx I can confirm that I'm seeing the same when deploying to Vercel. At this point, I'll assume that the correct header will be set, although I haven't found any way to confirm it locally. (Still seeing only the font optimization headers when running in dev.)

Thanks for looking into this!

samcx commented 2 months ago

@zenzen-sol I am also seeing that on next dev.

I think what is probably happening is these i18n headers are working differently in next build. (locally using next build, it shows).

It's tough to say why it gets flipped and explain why next/font doesn't show on next build, but I do know React is making these decisions to best organize what to preload first.

CleanShot 2024-08-22 at 15 14 28@2x

In the meantime, I will be closing this issue!

amannn commented 2 months ago

Hey @samcx, I hope it's ok for me to chime in.

I've tested with the latest canary and am observing the same behavior as you do: In development, only the React optimization header value for Link shows up and after a production build, only a manually set value for Link is returned (in this case set by next-intl).

There are two aspects here that worry me a bit:

  1. Even when I bump reactMaxHeadersLength to a value like 999999999, the behavior is exactly the same. Could it be, that a logic for merging headers is missing here? That's at least what I'd hope to exist and I guess there's a need for both: proper i18n links as well as performance optimization.
  2. A Link response header being overridden in development is quite unfortunate. While I'm happy that it still exists in production, I'd assume this to be rather confusion for developers who want to verify the header is there. Note that the Link response header in the i18n use case is not an optimization, but a necessary signal to search engines to link together multilingual pages. The absence of this header could result in search engines recognizing variants of pages as duplicates and de-indexing them. Therefore this is quite a crucial functionality.

Any chance we could get to the bottom of this and see if there's something to be adjusted in Next.js to work correctly?

samcx commented 2 months ago

@amannn Thank you for the clarification! I should have also checked next-intl when next/font wasn't present. When next/font isn't present, the next-intl Link Headers do show up in next dev.

Not sure if it's getting "replaced," but I have shared this with our team!

amannn commented 2 months ago

Awesome, thanks a lot @samcx!

amannn commented 3 weeks ago

@samcx I've just tried out Next.js 15 RC 2 and this issue is still present. Are you working on a fix? I also noticed this affects next/image if priority is used.

I've simplified the reproduction further:

  1. 0af3725: Basic app setup with an app that returns a link header from the middleware. Screenshot 2024-10-16 at 12 48 16
  2. 63018b1: Once a font from next/font is used, alternate links from the middleware disappear (both in dev and prod). Screenshot 2024-10-16 at 12 48 33
  3. 48ad20d: Once a next/image with priority is used, again the alternate links from the middleware disappear. Note however, that they compose with the ones from next/font. Screenshot 2024-10-16 at 12 48 46

The expected behavior would be that the link header that is set in the middleware should always be preserved.

Note that the link header can be of critical importance for sites that use this mechanism to instruct search engines about pages being available in multiple languages. If the header is missing, it can result in search engines interpreting the variants as duplicate content.

In Next.js 14 this works fine, please ensure a smooth upgrade path for users who rely on this mechanism 🙏 (i.e. most of the people using next-intl).

amannn commented 2 weeks ago

@samcx Unfortunately, this is still the case in the stable release of Next.js 15. Are there plans to fix this? As mentioned in my previous comment, this header is critical for search engines to link content correctly, avoiding marking content as duplicate.

samcx commented 2 weeks ago

@amannn There are still plans to fix this, we just did not get to this issue yet!

We will get back to you when we have an update on this.

amannn commented 2 weeks ago

Got it, thanks!