-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Allow static exports without trailing slashes #3283
Allow static exports without trailing slashes #3283
Conversation
@gkaemmer awesome, I'll review this latest this week 👍 |
@timneutkens any updates on when this might be released? |
any updates on when this might be released? |
Hello, any update on when this might get merged & released? It's a pretty crucial feature for my use case of next's static export (I'm currently solving this by commenting out the relevant lines in the node_module, which isn't really great 🙁) |
4247099
to
d62d9d1
Compare
Was this working for you @gkaemmer? Looks like you meant I noticed that routes without trailing slashes worked on a static site until I imported |
No, actually, I started trying to fix tests yesterday then got sidetracked--not ready to be merged yet :( |
Just skimming the build logs, looks like |
That's the only lint error, but I think there might still be some failing tests after it passes linting Running tests locally now, they take quite a long time. |
d62d9d1
to
44bc642
Compare
|
||
// By default, append a trailing slash if this path does not have an extension | ||
const exportTrailingSlashes = __NEXT_DATA__ && | ||
__NEXT_DATA__.nextExportTrailingSlashes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I was thinking this morning that this might actually be compiled at build time instead of providing it at runtime and having __NEXT_DATA__
used here.
So what you could do is add DefinePlugin
with process.env.NEXT_EXPORT_TRAILING_SLASHES
which holds the value of config.exportTrailingSlashes
. So that it's compiled at build time.
I'm also not sure how you're passing the config to the client right now, so it wouldn't work client-side right 🤔 (with the current solution)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented in #4524
This is fixed 🎉 |
Amazing! Thanks @NathanielHill |
I'm still not sure how now static deployments fixes this 🤔 |
Seems like this won't fix the problem in case you're not deploying to Now. |
So is this officially marked as solved? Do people who don't deploy to Now still have to solve this issue by commenting out pieces of code in the next.js module? 🙁 |
Like I said:
|
This looks like a ugly workaround, but I've set |
@dasbego I've tried your fix and it seems to work well enough. The links still have the trailing slashes on hovering (and displaying the href), but on clicking they actually lead where they're supposed to (no trailing slashes). Is this happening to you too? |
@bozskejkaja I just tried the NEXT_DATA.nextExport = false workaround and it seems to work as advertised. I don't see the trailing slash in mouseover. Maybe you are seeing some cached resource? |
Can this or something similar be re-opened for folks who are using |
This is an annoying little bug. All we need is a little config flag to switch this on and off... |
@justinphilpott it's on experimental features already. |
@timneutkens in const withCSS = require('@zeit/next-css')
const nextConfig = {
exportPathMap: async () => {
return {
'/': { page: '/' }
}
},
experimental: { exportTrailingSlash: false }
}
module.exports = withCSS(nextConfig) |
Hello, first PR here 😎. Please let me know if anything should change.
Fixes #2944 by adding a
exportTrailingSlashes
option tonext.config.js
.I'm not crazy about adding to the global
__NEXT_DATA__
object, but it seems the cleanest way to get settings so deep into the rendering logic.I looked into adding a test for this, but it would seem to add an ugly wart to
integration/static/test/index.test.js
, which only tests a singlenext.config.js
. Can definitely add if necessary though.