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

Fix sitemap generation if next/headers is used #759

Closed
wants to merge 1 commit into from

Conversation

jlubawy
Copy link

@jlubawy jlubawy commented Jan 18, 2024

I was looking into issue #692 that I've also seen, if a page uses next/headers then the generated sitemap files are empty.

Admittedly I don't understand the NextJS internal files that are created, but I did find that:

  1. When headers() is not included (normal behavior): page routes seem to show up in the .next/prerender-manifest.json file
  2. When headers() is included: prerender-manifest.json is mostly empty, but there is another file .next/trace that seems to include the missing paths. trace is a line-delimited JSON file, each line contains an array of objects with an optional field tags.path containing the missing paths

This PR adds parsing of the trace file and addition to the manifest. It then uses this as input when building the URL set. I tested against the repro mentioned in #692 (https://github.com/bastienrobert/next-sitemap-generation-with-headers-repro) and it appears to fix the problem.

The parsing of the trace file could probably use some work (scanning each line instead of splitting on \n), but I wanted to get this PR up to get some feedback to make sure I'm on the right track.

Copy link

vercel bot commented Jan 18, 2024

Someone is attempting to deploy a commit to a Personal Account owned by @iamvishnusankar on Vercel.

@iamvishnusankar first needs to authorize it.

Copy link

Closing this PR due to inactivity.

@jlubawy
Copy link
Author

jlubawy commented Mar 19, 2024

FWIW this is no longer an issue for us, I'm not exactly sure why, but either it was fixed upstream in NextJS, something like this https://github.com/pacocoursey/next-themes?tab=readme-ov-file#avoid-hydration-mismatch, or our code removed the use of next/headers that was causing problems.

I can no longer reproduce this: #692

Closing this PR.

@jlubawy jlubawy closed this Mar 19, 2024
@tiste
Copy link

tiste commented Apr 24, 2024

@jlubawy on my side, I still have this issue with last version of Next.js (14.2.1), et next-sitemap (4.2.3), with headers next/headers.

@jlubawy
Copy link
Author

jlubawy commented May 15, 2024

Basically what we did was wrap the code that caused the issue in a client-side only hook similar to what is mentioned here vercel/next.js#2473 (comment)

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

Successfully merging this pull request may close these issues.

None yet

2 participants