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

Inconsistent behavior between dev and prod for link href #1568

Closed
3 tasks done
damianfrizzi opened this issue Nov 21, 2024 · 12 comments · Fixed by #1571, #1573, #1576 or #1578
Closed
3 tasks done

Inconsistent behavior between dev and prod for link href #1568

damianfrizzi opened this issue Nov 21, 2024 · 12 comments · Fixed by #1571, #1573, #1576 or #1578
Labels
bug Something isn't working unconfirmed Needs triage.

Comments

@damianfrizzi
Copy link

damianfrizzi commented Nov 21, 2024

Description

The href generated by the next-intl Link component is not consistent between dev and prod and produces wrong href results.

My next.config contains trailingSlash: true which might causes some issues.

Verifications

Mandatory reproduction URL

https://next-revalidate-bug.vercel.app/

Reproduction description

Source code

To reproduce issue 1

  • Go to demo and check the href of the "FR" link under "Client Component"
  • It shows /fr/de-ch/ instead of /fr/
  • Click "Go to some page" and then "Go to homepage"
  • The "FR" link under "Client Component" now shows "/fr/"
  • This issue happens only when running bun run build && bun run start and when deployed to Vercel

To reproduce issue 2

  • Go to demo and check the href of the "DE" link under "Client Component" or "Server Component"
  • It shows /de/ even though this is the default locale and shouldn't be shown with the locale prefix mode as-needed

Expected behaviour

  • The default locale with mode as-needed should never be part of the href
  • No incorrect href is created (e.g. /fr/de-ch/)
  • The behaviour between dev and production should be consistent
@amannn
Copy link
Owner

amannn commented Nov 22, 2024

Hey, thanks a lot for the careful reproduction @damianfrizzi!

Issue 1

Ough, that was a tricky one to diagnose. I found that there's an upstream bug in Next.js that's causing this: vercel/next.js#73085.

I think I can add a workaround in next-intl though to address this: #1571. Not sure when/if this will be addressed on the Next.js side, so that should hopefully help as an immediate fix.

I'll see if CI is happy and will have another look at this early next week to make sure the workaround doesn't have any unforeseen consequences. If this issue is urgent for you, you can use href="/" in the locale switcher for the time being.

Issue 2

That's probably because you're using a wrong env param here:

domain: process.env.NEXT_PUBLIC_VERCEL_ENV ?? 'localhost:3000'

See the docs for NEXT_PUBLIC_VERCEL_ENV.

Related to this, see using domains with localePrefix: 'as-needed' in the docs.

@damianfrizzi
Copy link
Author

Thanks @amannn for your quick investigation and solution 🙏

Regarding issue 2, I do indeed have the wrong env 🤦 I guess I messed that up when creating the minimal repro (as it does work correctly on my main project).

I look forward to testing your fix when it comes out in a new version! I also have slight hopes that it might also fix another issue I found in the same demo (reported here vercel/next.js#73013).

@amannn
Copy link
Owner

amannn commented Nov 22, 2024

Sure, happy to help! 🙂

I look forward to testing your fix when it comes out in a new version! I also have slight hopes that it might also fix another issue I found in the same demo (reported here vercel/next.js#73013).

Hmm, interesting, I haven't encountered that one so far. There's one mention on revalidatePath in the docs that was contributed by a user, but that seems to be a different story …

@amannn
Copy link
Owner

amannn commented Nov 25, 2024

@damianfrizzi Ok, the fix looks good to me! I've tried out the just-released next-intl@3.25.2 in your reproduction and the bug appears to be fixed.

Can you try the fix out in your app?

Thanks for the great reproduction!

@damianfrizzi
Copy link
Author

Hey @amannn, thanks for the quick release!
I've just deployed 3.25.2 in my reproduction and the bug appears to be fixed on the root page but not on the sub page. If I navigate to this page and check the "DE" href I still see a wrong href (/de/fr-ch/somePage/).

@amannn
Copy link
Owner

amannn commented Nov 26, 2024

Oh I think my condition was a bit too strict, I can also reproduce it here: https://next-revalidate-bug.vercel.app/fr/. The culprit is when a non-default locale is used and the page renders on the server (navigating on the client side avoids the issue).

Should be a quick fix, will follow up shortly!

@amannn
Copy link
Owner

amannn commented Nov 26, 2024

Ok, this should be fixed in next-intl@0.0.0-canary-b73586f. I've upgraded your reproduction locally and the error seems to be gone now.

Can you try out the canary release in your app? If everything looks good, I'll release this to stable.

@damianfrizzi
Copy link
Author

Just updated the repro to the canary release and it looks good now 🎉

@amannn
Copy link
Owner

amannn commented Nov 26, 2024

Great! Does it also work correctly in your actual app now?

@damianfrizzi
Copy link
Author

Just tested and it also works in my actual app 👍

Side note: Before your fixes, the default locale was never in the href's and now it is. I.e. it now behaves according to the special case described here.

@amannn
Copy link
Owner

amannn commented Nov 26, 2024

That sounds good, thanks!

@amannn
Copy link
Owner

amannn commented Nov 26, 2024

Oh wow, just received the notification about your sponsoring—thank you so so much, I really appreciate it @damianfrizzi! ❤️

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