Unknown routes should render 404.html

[gamliela]

Hi,

Cool project! I was looking for something similar for quite long, and your solution seems to work pretty well!

The current implementation doesn't support rendering of the 404.html file, if an unknown URL is requested. For example, try this link: https://next-on.netlify.com/some-non-existing-url - it should actually render _next/pages/404.html file.

A quick fix that I've found for this problem is to simply copy the 404.html file to public in a post-build step. Something like cp public/_next/pages/404.html public. The reason is that Netlify are expecting to find this file exactly on this place. But I wonder if that's the right solution, or if there are better ways. Another approach that I've tried was to add /* /_next/pages/404.html 404 at the end of _redirects file, but it didn't work very well with netlify-dev.

Would love to hear your thoughts. Thanks :)

[FinnWoelm]

Hi @gamliela, thank you for opening this issue and for sharing solutions of your own!

I will look into it this week and get back to you soon. I'd be quite interested to hear how you ended up dealing with it after all: By copying the 404.html to public after building or by adding the redirect?

PS: Apologies for taking so long to get back. Somehow it totally slipped my attention.
PPS: I accidentally tagged this issue in the latest release, when I meant to tag the second pull request (#5) instead. 😶

[gamliela]

No worries :) Yes, I simply copy the 404.html file. Redirect didn't work well for me. Just thought it could be nice to add that feature, since it's already moving a lot of files around.

[FinnWoelm]

Hmmm, very true. I like the idea!

Two considerations:

• If there is already a user-created 404.html file in the public folder, we don't want to overwrite that. We skip copying the 404.html in that case and print out a warning?

• If the NextJS 404 page is dynamic (uses getInitialProps), we need to route the request to a Netlify function. So in this case, we need to add that redirect you mentioned. This will break locally previewing with netlify dev, so we need to figure something out... Any ideas? I wish netlify dev was consistent with Netlify in how it handles redirects.

[gamliela]

Good points!

If there is already a user-created 404.html file in the public folder, we don't want to overwrite that. We skip copying the 404.html in that case and print out a warning?

Probably not overriding - what is the existing behaviour (for example when generating the _redirects file?)

If the NextJS 404 page is dynamic (uses getInitialProps), we need to route the request to a Netlify function. So in this case, we need to add that redirect you mentioned. This will break locally previewing with netlify dev, so we need to figure something out... Any ideas? I wish netlify dev was consistent with Netlify in how it handles redirects.

I am not sure that 404 page can be dynamic, it is a safe fallback mechanism that is supplied by Nelify (not next.js) when no other route could be found. As Netlify not aware to dynamic content, I'm not sure you can make this page dynamic. I tried catch-all rule at the end of the routes file, but I don't remember now if it ever worked for me...

[FinnWoelm]

Hey @gamliela,

This is finally being fixed in the upcoming next-on-netlify v2.

I've already written the code and tests; it works wonderfully. There are a few more things I want to look into before releasing v2, but I'm getting very close!

Finn 🙂

PS:

I am not sure that 404 page can be dynamic, it is a safe fallback mechanism that is supplied by Nelify (not next.js) when no other route could be found. As Netlify not aware to dynamic content, I'm not sure you can make this page dynamic. I tried catch-all rule at the end of the routes file, but I don't remember now if it ever worked for me...

You were totally right on this. I was confusing it with the pages/_error.js page.

[gamliela]

Thanks for the update! great news :)

I've tested wip-version2 branch. I guess it's still work on progress, but if you want I can share some of my findings.

1. I got error Error: 'out/index.html' already exists when I use /pages/index.jsx file. When I rename the file to /pages/index2.jsx it works;
2. I got 404.html generated on the project root, but also on /out directory. Is this by intent? I would suggest to avoid generate files on the project root as default behaviour;
3. Maybe worth to update documentation about what should be added to .gitignore. Not sure about that, but at least in my V1 configuration I had this block:

# auto-generated files by build/next-on-netlify
/functions/nextRouter
/public/_next
/public/404.html
/public/_redirects

My test project is at https://github.com/gamliela/playground-auth/tree/next-on-netlify-v2

Thanks for the awesome work! looking forward seeing the final version :)

[FinnWoelm]

Hey @gamliela,

Wow, thanks for checking it out so quickly and looking for issues!

1. OH!! You are right. When I copy the pre-rendered pages to out/, I do a check to make sure that the file does not already exist. This is to make sure that I don't silently overwrite one of the files copied from public/.

   But for index.js files, Netlify always creates two page entries, one routing to / and one routing to /index. So in this case, the first copying of index.html to out/ goes fine. The second copying fails because index.js already exists.

   I am so glad you caught that. In my tests, the index.js files were always using getInitialProps, so the issue didn't show up 😶 I just fixed it and added a test to make sure it doesn't happen again in the future! If you get another chance to have a look, let me know if it works now.

2. The duplicate 404.html page is on purpose. There is an inconsistency between deployed Netlify and netlify-cli dev. Currenly, Netlify (deployed) expects the 404.html file in the publish folder, while netlify-cli dev expects it at the directory root.

   In order to cover both cases, I'm copying the 404.html to both locations until this is fixed. I want to file an issue with https://github.com/netlify/cli, but haven't had a chance yet. Feel free to go for it, if you want 😀

3. Yes, things changed a bit! The wip-version2 README should be up to date: https://github.com/FinnWoelm/next-on-netlify/blob/wip-version2/README.md

   Notable changes:

   • The publish folder in netlify.toml should now be out/
next-on-netlify now copies pre-rendered pages and static assets and the contents of the public/ directory into out/. This way, we don't have to worry about messing up the public/ folders and it is consistent with what next export does.
   • For local previewing, you should use netlify-cli v2.51.0 or later. Lots of inconsistencies between deployed Netlify and netlify dev have been fixed!
   • You no longer need a 3rd party library such as serve or http-server for local previewing. netlify-cli comes with its own. But you do need to change the netlify.toml configuration:

     [dev]
       functions = "functions"
       publish   = "out"
       # We manually set the framework to static, otherwise Netlify automatically
       # detects NextJS and redirects do not work.
       # Read more: https://github.com/netlify/cli/blob/master/docs/netlify-dev.md#project-detection
       framework = "#static"

   • If you do local previewing, you .gitignore should now cover:

     # Files generated by next-on-netlify command
     functions/nextRouter
     out/
     404.html

Again, thanks for being so quick to take a look! 🙌 You definitely caught a major issue there! Keep up the search for more 😁

I am going to look into getStaticProps and getServerSideProps now and see if there is something we can do.

[gamliela]

1. Great!
2. Sounds like an issue, not sure about this workaround, let's hope it will be resolved soon.
3. I wasn't aware to all these changes. It seems that I need to read the README again :)

I'll continue playing with it and let you know if I find anything else. Thanks!