π BUG: Astro.url.pathname is missing trailing slash on prod build
What version of astro are you using?
1.0.0-rc.7
Are you using an SSR adapter? If so, which one?
none
What package manager are you using?
yarn
What operating system are you using?
archlinux
Describe the Bug
I just upgraded to 1.0.0-rc.1 and got a warning about Astro.canonicalURL.pathname (which I am using to highlight selected menu).
I replaced its usage with Astro.url.pathname and everything looked fine with astro dev.
However when doing the static build with astro build the menu does not get highlighted.
With some debugging, it turns out, that Astro.url.pathname:
- is the exact pathname on
dev(with a trailing slash if the URL has one - and not otherwise) - but on
buildthere is never a trailing slash (even if the URL has one - somehow expected, since the sameindex.htmlfile is served)
Looking at the (now deleted) code of createCanonicalURL https://github.com/withastro/astro/pull/3959/files#diff-7d09784396a04fa12dd4dcfbe9d6f7528bfe522487493851dc08957000481facL6 the added trailing slash has been dropped.
A proper fix would:
- adjust this URL on
devto match the URL which would be used forbuild(with a trailing slash to match the behavior before RC, without if you think this would be better) - be documented on the migration guide
What do you think?
Link to Minimal Reproducible Example
https://stackblitz.com/edit/github-mz4j6y?file=src/pages/test.astro
Navigate to the following URLs:
- https://github-mz4j6y--3000.local-corp.webcontainer.io/test (pathname:
/test<- will be the onastro build) - https://github-mz4j6y--3000.local-corp.webcontainer.io/test/ (pathname:
/test/)
Participation
- [x] I am willing to submit a pull request for this issue.
related to #3959 by @FredKSchott (reviewed by @matthewp and @tony-sull), who may have an opinion about this
Thanks for opening an issue! This definitely sounds like a problem.
If you're willing to open a PR, we'll definitely make time to review it. Otherwise we'll try to get this prioritized as soon as we can.
@natemoo-re thank you for the feedback.
Which solution do you think is the best?
- add a trailing slash, when the URL should have one (not easy to implement, but similar to the behavior "pre-RC")
- remove the trailing slash, except on root (much easier to implement, but a breaking change)
I would prefer the second option, since it is more consistent and easier to implement correctly.
I just wanted to say that I hit this bug as well just now. I have some logic for my nav links that compare the pathname from the current page to the href of the link, and Percy.io noticed that my active page link was no longer being highlighted.
I think now that 1.0 has been released, the first option has to be used to avoid a breaking change, right? Otherwise, folks like me would need to go through and update the logic to remove trailing slashes, and not expect them even when they're in the url in the browser.
avoid a breaking change
Since the current behavior (as of 1.0) is to have the paths without trailing slash on astro build, this would mean fixing the astro serve to always provide the path without trailing slash.
(so 1.0 was a breaking change and we now have to change βserveβ to be coherent)
Oh interesting, I'm still getting trailing slashes in 1.0.5...
@IanVS is your issue getStaticPaths related?
@oliverpool in dev Astro.url.pathname should be matching what you typed into the browser. If that is not the case then please let me know.
Build mode is trickier to answer If you have always then it should be adding them, but otherwise I'm not sure.
@matthewp no I'm not using getStaticPaths for this. I have simply:
const currentPage = new URL(Astro.url.pathname, Astro.site).pathname;
in my astro layout's frontmatter.
On a page like http://localhost:3000/nebula/config/ the currentPage is:
/nebula/configin production/nebula/config/in dev
In both cases, the url is the same in the address bar. And I don't have trailingSlash set in my config.
@IanVS Thank you for the specifics!
Correct me if I'm wrong, but does /nebula/config and /nebula/config/ both work in dev?
In dev, the currentPage matches the url both with and without the slash. So:
Dev:
http://localhost:3000/nebula/config/-> /nebula/config/http://localhost:3000/nebula/config-> /nebula/config
Prod:
http://localhost:3000/nebula/config/-> /nebula/confighttp://localhost:3000/nebula/config-> /nebula/config
Is that what you were asking?
I think so. Is the file src/pages/nebula/config.astro?
The new URL(Astro.url.pathname, Astro.site).pathname is in the layout. But the behavior I'm seeing happens for pages created both by /src/pages/nebula/[slug].astro and /src/pages/nebula/index.astro
@matthewp I think everything is documented into my first post (with a Minimal Reproducible Example):
dev Astro.url.pathname should be matching what you typed into the browser
It does. But my main issue is with astro build where the slash is always dropped (it is expected that it has the same value when requested with a without a trailing slash, since this is static. However it does not match the astro dev behavior).
Maybe the trailingSlash setting should be set to 'never' by default to somehow match the astro build behavior?
To sum up, my issue is that astro dev and astro build behave differently regarding Astro.(canonical)URL.pathname, which wasn't the case before 1.0.0-rc.1 (#3959 changed the logic without warning).
Thanks for all of the feedback! I agree that there's a problem here, just trying to understand the nuance in order to find the correct solution.
build can only create pages once, so it has to choose either to add the trailing slash or not. Since the default trailingSlash is ignore then it doesn't know which behavior you want.
As you pointed out, Astro.canonicalURL didn't have this problem because it added a slash. Since that API is gone we are getting a behavior from Astro.url. I think the solution is that the build should probably add a trailing slash by default (if trailingSlash is not set to never) which would match our previous default.
Let me know if this would work for you @oliverpool @IanVS. I'm going to consult with the core team to see if this idea makes sense to them as well.
Yep, that would work great on my end, thanks.
Sounds good to me. A couple of remarks:
- As far as I understand
trailingSlashis only for thedev server. I don't see why it would impactastro buildin any way. Its goal is only to match the behavior of the actual static server that will be used on production. - maybe the
Astro.urlshould depend on build.format:directory=> with trailing slash (default)file=> without trailing slash
Yeah that's an interesting point.
Talked about this with core and we are not comfortable with changing the defaults now that 1.0 is out. Even though I don't like the mismatch here either, some users are likely depending on the default-no-slash behavior that currently exists.
What should be working is that if you set trailingSlash: 'always', that should force you to use a trailing slash in dev and also it should be added in the build. I'm going to write a build test to confirm that it's working, as I'm not sure it currently is.
Also the docs will be updated because although they say the config is for the dev server, it's a top-level config and should be applied to both dev and build.
If that's the case, it might also be good to note that the replacement of Astro.url for cannonicalURL is not a direct replacement, and this difference must be accounted for when changing from one to the other.
we are not comfortable with changing the defaults now that 1.0 is out
Too bad that this issue got attention after the 1.0 release (this issue was opened 2 days before the release).
If you don't want to change it, it means that you consider the current behavior to be working as expected? (for me the current behavior is broken)
There were a lot of changes between the late betas and the final release, which went very quickly. I'm really surprised how many breaking changes were made late in the game. Definitely a bummer that there wasn't more time for community feedback. π
To be fair, I don't think there's any breaking change here. Astro.url was a new feature and as far as I'm aware, we didn't change its behavior. It's just not as clean of a replacement for Astro.canonicalURL as we had thought.
The mistake, to me, is making ignore the default for trailingSlash. The build has to either have a trailing slash or not have one, we can't "ignore" it, so this creates an unfortunate dev/build difference.
We can and should fix this in 2.0. I'm not closing this issue until we have that figured out. A 2.0 is not necessarily a long way out.
I do take all feedback about the instability in the beta/RC period. We didn't do a great job of planning on breaking changes and then make them all at once; instead we allowed ourselves to continue to find things we wanted to change until close to the 1.0 release. We've heard this feedback from a lot of the community (and among ourselves) and will work to not do this the next time around.
Part of that is not breaking apps now, though. So as much as I personally do not like the current behavior, breaking it for everyone else today is not the right answer. I think the right answer is to make sure trailingSlash: 'always' works everywhere, and possibly that is what we should encourage people to use.
Ok, the team has been talking about this again and now I think our opinion is changing based on the build.format argument you made @oliverpool. We are using this information to determine where to place the file but are not using it to determine the request URL and that feels wrong. Since you've chosen directory you should be able to resolve relative URLs as those you are within a directory. We are still talking about it but are coming around. cc @IanVS
Ok, we do have consensus now. We will make the build follow build.format so that if it is set to directory (the default), your url will be /page/ and if it is set to file it will be page.html. This should make relative URLs created using Astro.url work as you would want/expect.
Thanks to you both @oliverpool @IanVS for making a strong case here and being patient with me. I hope to get this into a patch fix soon.
PR: https://github.com/withastro/astro/pull/4352
https://github.com/withastro/astro/issues/4404 @FredKSchott Thanks for your reply, I tried this configuration, but I didn't get the results I wanted.
My current problem is that when reading the astro.config.mjs file, the jump in the link component gives an extra / result, but not this.
