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

trailing slash in link for legit page works for client side navigation but leads to not found bundle and 404 on hard refresh (ssr) #5214

Closed
iamstarkov opened this issue Sep 19, 2018 · 124 comments · Fixed by #13333
Assignees
Milestone

Comments

@iamstarkov
Copy link

trailing slash in link for legit page works for client side navigation but leads to not found bundle and 404 on hard refresh (ssr)

Bug report

Describe the bug

let me know if title needs further clarification.

all relevant issues has been closed with reasoning that its been fixed in 6-canary (I believe it is not) or by improved serve (which is true only in perhaps production static export).

I'm rewriting my existing blog to next.js and i previously used trailing slashes. Latest serve can help with it once i build my next.js powered blog. But in order to fix dev env i need either to get rid of trailing slashes and utilize 301 Moved Permanently in prod; or live with broken trailing slash support in dev.

To Reproduce

Here is minimal reproducible case (link to repro repo is below snippet):

// pages/index.js
import Link from "next/link";

export default () => (
  <Link href="/about/">
    <a>About</a>
  </Link>
);

// pages/index.js
export default () => "about";

Minimal reproducible repo https://github.com/iamstarkov/next.js-trailing-slash-bug-demo

  1. clone repo git clone https://github.com/iamstarkov/next.js-trailing-slash-bug-demo
  2. change directory cd next.js-trailing-slash-bug-demo
  3. install deps yarn
  4. run dev: yarn dev
  5. open http://localhost:3000/
  6. open devtools' network tab
  7. observe http://localhost:3000/_next/static/development/pages/about.js being 200ed
  8. observe http://localhost:3000/_next/on-demand-entries-ping?page=/about/ being 200ed
  9. observe http://localhost:3000/about/ being 404ed
  10. observe persistent attempts to resolve http://localhost:3000/about/
  11. observe in the terminal Client pings, but there's no entry for page: /about/
  12. refresh the page
  13. observe 404 page.
  14. remove trailing slash in the url or click http://localhost:3000/about
  15. observe page being 200ed
  16. to ensure error persistence repeat steps 5-15 once.

Expected behavior

  1. /about/ shouldnt be resolved as 404 not found
  2. /about/ should be resolved as 200 ok
  3. Server should not print Client pings, but there's no entry for page: /about/
  4. both /about and /about/ should work the same way

Screenshots

N/A

System information

  • OS: macOS High Sierra 10.13.6 (17G65)
  • Browser (should not matter, but can repro'ed in chrome 69.0.3497.100 and safari Version 12.0 (13606.2.11) (was the same for safari 11)
  • Version of Next.js: 7.0.0 (could repro on 5.x and 6.x)

Additional context

Add any other context about the problem here.

If you change this code in https://github.com/zeit/next.js/blob/459c1c13d054b37442126889077b7056269eeb35/server/on-demand-entry-handler.js#L242-L249

or node_modules/next/dist/server/on-demand-entry-handler.js locally

          const { query } = parse(req.url, true)
          const page = normalizePage(query.page)
+         console.log('query.page', query.page);
+         console.log('page', page);
+         console.log('Object.keys(entries)', Object.keys(entries));
          const entryInfo = entries[page]

          // If there's no entry.
          // Then it seems like an weird issue.
          if (!entryInfo) {
            const message = `Client pings, but there's no entry for page: ${page}`

and restart next dev and open http://localhost:3000/ and click about link then:

  • for /about
    query.page /about
    page /about
    Object.keys(entries) [ '/', '/about' ]
    
  • for /about/:
    query.page /about/
    page /about/
    Object.keys(entries) [ '/', '/about' ]
    Client pings, but there's no entry for page: /about/
    

I think the problem (at least part of it) is in inability of onDemandEntryHandler's middleware to find page in entries if page has trailing slash.

I hope my 2 hours of investigation and preparation can help with fixing this issue.

@iamstarkov
Copy link
Author

iamstarkov commented Sep 19, 2018

most relevant and notable issues are #1189 and #3876

@NathanielHill
Copy link
Contributor

Looking forward to this being finally resolved! @timneutkens What's the status of trailing slash issues for Next 7?

@iamstarkov
Copy link
Author

@NathanielHill I could reproduce it on next@7

@paintedbicycle
Copy link

I'm using nextjs 7 and trailing slash is producing a 404 for me on both dev and prod:

  • on initial page load
  • on page refresh

And affects:

  • an external link
  • an internal link
  • URL pasted into the browser

Simply removing the trailing slash fixes the issue.

Trailing slashes are often added by browsers, servers and/or other services where links might be pasted so while I can control internal links, it's hard to control what at what links external users might be arriving on

@emplums
Copy link

emplums commented Sep 27, 2018

I'm also seeing this issue in version 7. Not sure if this is relevant but, I'm aliasing one Next.js project to a subfolder of another Now deployment. So our base url is primer.style and we are aliasing our primer-components.now.sh Next.js app to primer.style/components. On production, primer.style/components's index page works fine, but primer.style/components/ produces a 404.

@chibicode
Copy link
Member

I had to search around a bit to find this issue. I use static deployments on Netlify so it's not an issue on prod, but on development (Next 7) the compilation just freezes if there was a trailing slash, and it was hard to figure out why. I don't think this (not handling trailing slash on dev environment) is a good DX.

@ravener
Copy link

ravener commented Nov 12, 2018

Im also having this issue and it is really annoying, i hope it is fixed soon.

@aluminick
Copy link

aluminick commented Dec 7, 2018

If you want trailing slash, you can just do this. <Link href='/about' as='/about/'><a>about</a></Link> but if you're using fridays/next-routes this is not possible. So I have a fork where you can add trailingSlash as prop. Hope this helps

@iamstarkov
Copy link
Author

If you want trailing slash, you can just do this. <Link href='/about' as='/about/'><a>about</a></Link> but if you're using fridays/next-routes this is not possible. So I have a fork where you can add trailingSlash as prop. Hope this helps

@aluminick I'm sorry, I just tried this and it doesnt work for me. I still get to traling-slashed page (latest release), which is not found after refresh (current behavior).

@iamstarkov
Copy link
Author

also neither #6664 nor #6752 help with these, because experimental.exportTrailingSlash doesnt help because it is for next export only, I believe

@iamstarkov
Copy link
Author

there was a promising pull request #6421 by @Janpot which didn't reach any consensus, unfortunately

@dryleaf
Copy link

dryleaf commented Jul 15, 2019

@iamstarkov What's the status of this issue? Any solutions beside the server.js hook?

@iamstarkov
Copy link
Author

@dryleaf status: it is still opened

@jabacchetta
Copy link

jabacchetta commented Jul 15, 2019

A similar issue... redirect when multiple forward slashes are added. Example: https://github.com/zeit/next.js////////////issues/5214

@iamstarkov
Copy link
Author

GitHub urls are irrelevant

@jabacchetta
Copy link

jabacchetta commented Jul 15, 2019

@iamstarkov Not sure what you mean. But after rereading my original post, it looks like I could have been more clear.

The GitHub url is meant to be a simple demonstration of how urls should (preferably) work when an app is built with Next.js. In other words, if a user adds an extra slash, the url should still work.

@Timer Timer modified the milestones: 9.x, 9.0.x Jul 15, 2019
sudkumar pushed a commit to sembark/gladio that referenced this issue Aug 3, 2019
@Timer Timer modified the milestones: 9.0.x, backlog Aug 30, 2019
@farhadniaz
Copy link

Any update for nextjs 9 ?

@nik-john
Copy link

I'm new to Next but what is the workaround you folks are using for this issue?

@untitledlt
Copy link

How long does it take for feature to get from canary to master?

@Valnexus
Copy link

Valnexus commented Jul 1, 2020

This has now been resolved in next@^9.4.5-canary.17!

And how exactly it is resolved? just removing the trailing slash? if i access "www.site.com/help/" i get redirected to: "www.site.com/help" , can we have an option there we opt for leaving ending slash? accessing "www.site.com/help/" or "www.site.com/help" will leave or redirect or add "/" at the end to have: "www.site.com/help/"

@timneutkens
Copy link
Member

timneutkens commented Jul 1, 2020

@Valnexus see #13333, it includes an experimental option:

module.exports = {
  experimental: {
    trailingSlash: true
  }
}

How long does it take for feature to get from canary to master?

When it's ready. There are still edge cases in the handling that are being solved. Once those have been fixed it can go to stable.

@armspkt
Copy link
Contributor

armspkt commented Jul 2, 2020

@timneutkens @Janpot

I tried the latest next canary (9.4.5-canary.27) but when I create test page and I access www.example/test/ it redirects to www.example/test
I think behavior for both cases should be the same.

When access www.example/test/ it should stay on www.example/test/.
When access www.example/test it should stay on www.example/test.
I test it on Nuxt.js, it works the same behavior that I describe above.

@Janpot
Copy link
Contributor

Janpot commented Jul 2, 2020

I think behavior for both cases should be the same.

The reason for a redirect is to make sure search engines don't see duplicate content. What's your exact use-case?

@hugomn
Copy link

hugomn commented Jul 2, 2020

I don't see why it's a closed issue if it isn't merged to a stable release yet. If I understood correctly it's only fixed in the canary release for now, right?

@Timer
Copy link
Member

Timer commented Jul 2, 2020

Issues are closed when their associated pull request lands, as they're available for immediate use on canary. If you need this feature, please upgrade to the canary channel.

@hugomn
Copy link

hugomn commented Jul 2, 2020

Sounds good. Thanks, @Timer!

@armspkt
Copy link
Contributor

armspkt commented Jul 2, 2020

@Janpot I saw https://github.com/issues/ and https://github.com/issues can access the same behavior without a redirect.

https://twitter.com/explore/ and https://twitter.com/explore, this one too.

If it has a problem with search engines, Why Github and Twitter did not fix it?
I think it is the default behavior for any website.

There is no specific use-case, it just my opinion that it should work that way.

@ziserman
Copy link

ziserman commented Jul 2, 2020

If it has a problem with search engines, Why Github and Twitter did not fix it?

@armspkt It is not a problem since there are several ways to solve it. For example Twitter uses <link rel="canonical"> attribute to tell search bots which page they should crawl and other versions should be marked as duplicated.

So redirect is a viable way to make SEO on your website. You can read more info here.

@armspkt
Copy link
Contributor

armspkt commented Jul 3, 2020

@ziserman If we have several way to solve it, we should keep the same url without redirect for user experience.

@Janpot nuxt-modules/i18n#422

Nuxtjs have several options to choose (undefined, true, false)

Should Nextjs have serveral options to choose too?

@ShahzadUmair
Copy link

ShahzadUmair commented Jul 3, 2020

The reason for a redirect is to make sure search engines don't see duplicate content. What's your exact use-case?

@Janpot Our API has trailing slashes in a lot of places. The Latest release raises a lot of 404's on the backend since the Urls with trailing slashes (/api/test/ -> /api/test) do not match

@mlbonniec

This comment has been minimized.

@Timer
Copy link
Member

Timer commented Jul 13, 2020

@mlbonniec I've minimized your comment because it causes severe performance regressions in a Next.js app.

The latest next@canary version fixes this bug, please upgrade instead!

@mlbonniec
Copy link

@mlbonniec I've minimized your comment because it causes severe performance regressions in a Next.js app.

The latest next@canary version fixes this bug, please upgrade instead!

No problem!
However, I updated earlier, and that did not solve the problem.
With npm update

@Timer
Copy link
Member

Timer commented Jul 13, 2020

If the latest Next.js canary doesn't fix the bug for you, please open a new issue so we can take a look. 🙏

@omarryhan
Copy link

Quick question, how will projects with next export handle this change? By creating an entirely new page for each page for the trailing slash? I don't think an exported app can specify HTTP redirects (or rewrites).

@Timer
Copy link
Member

Timer commented Jul 14, 2020

Projects that use next export will have all of their <Link />s on the client-side correctly updated, but the server-side redirect will require manual configuration. Projects deployed with the serverless target or next start will configure these settings automatically.

@seagyn
Copy link

seagyn commented Jul 15, 2020

@Timer once this reaches a full release, would we still need to use the experimental option?

@timneutkens
Copy link
Member

@Timer once this reaches a full release, would we still need to use the experimental option?

No, would just be available as-is.

@imdongchen
Copy link

I guess the trailingSlash option won't work for next export? What's the best way to redirect /page/ to /page (or vice versa) in, say, github pages?

@timneutkens
Copy link
Member

I guess the trailingSlash option won't work for next export? What's the best way to redirect /page/ to /page (or vice versa) in, say, github pages?

As far as I'm aware github pages does not have a redirects feature. This does work out of the box on vercel.com though which is also free for hobby projects (like github pages is).

@yingqi-chen
Copy link

yingqi-chen commented Oct 12, 2020

Projects that use next export will have all of their <Link />s on the client-side correctly updated, but the server-side redirect will require manual configuration. Projects deployed with the serverless target or next start will configure these settings automatically.

Hi @Timer Can you explain more? How can I config manually? So here is my situation. On my website, I use next-i18next. After I deployed with next build && next export, all of the internal links work but when manually enter URL, NONE of them works and lead to 404 error. From here I decided to use trailingSlash:true and so manually enter /pricing will work now but /zh/pricing leads to 404 errors.

@SamSverko
Copy link

I guess the trailingSlash option won't work for next export? What's the best way to redirect /page/ to /page (or vice versa) in, say, github pages?

I'm in the same boat today. I'm using next export with GitHub pages for hosting.

  • Visiting www.website.com/about is fine
  • Visiting www.website.com/about/ results in a 404.

I still can't find a solution for this one.

@ACPK
Copy link

ACPK commented May 20, 2021

@SamSverko - Did you find a solution?

@SamSverko
Copy link

@ACPK I have not. I sort of gave up using GitHub pages for this.

@tomjowitt
Copy link

@SamSverko @ACPK next export is really tough to work with for any URL manipulation. Things like subfolders and trailing slash redirects simply don't work. We have a Cloudfront/S3 static site setup and the only way we could fix these issues was to run Lambda@Edge functions on each request and to fix things before they hit the next app. It's a real pain. You could also do it by putting Caddy or Nginx in front of your static site.

It's frustrating as other static site generators such as Hugo do this out of the box.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.