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

🐛 BUG: Generated relative paths for assets causes 404 pages to render incorrectly #2561

Closed
DaveOrDead opened this issue Feb 9, 2022 · 6 comments · Fixed by #3422
Closed
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@DaveOrDead
Copy link

DaveOrDead commented Feb 9, 2022

What version of astro are you using?

0.22.0

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

Hey team, thanks for the awesome project!

I noticed that because the paths for assets are generated as relative it causes 404 pages to render incorrectly when the url nesting is 3 levels+ deep. (Ironically it causes a 404 for all CSS and JS assets)

I discovered it on my own site, so then checked the Astro docs site and discovered it is happening there as well:

https://docs.astro.build/lost - ✅ 404 renders correctly
https://docs.astro.build/really/lost - ✅ 404 renders correctly
https://docs.astro.build/really/lost/help - ❌ 404 renders incorrectly and if you look in the network you can see the majority of assets cannot be found

2 levels
Screen Shot 2022-02-09 at 11 24 37 pm

3 levels
Screen Shot 2022-02-09 at 11 19 40 pm

I'm not sure why 3 levels is the magic number - but anything beyond that it fails.

One solution I thought of was that if the paths to those assets were absolute then they should always be correct but I'm not sure how easy that would be to implement?

Link to Minimal Reproducible Example

https://docs.astro.build/really/lost/help

@natemoo-re
Copy link
Member

I think we've discussed switching the default to absolute paths. Would love to see that happen, as well.

As a temporary workaround for 404 pages, we could use the <base> element.

@DaveOrDead
Copy link
Author

Ah good suggestion, thanks! We've ran into difficulties before with <base> where things like <a href="#cake"> won’t be #cake added to the current url, but added to the base url and ends up needing more workarounds.

For now I've scoped the <base> element to only render for our 404 page to avoid potential problems on other pages - which is working but I'm not sure if I've gone about it in the best way in Astro terms?

---
const { url } = Astro.request;
const urlSplit = url.pathname.split('/');
---

{
  urlSplit[urlSplit.length - 1] === '404'  && (<base href={`${import.meta.env.PUBLIC_WEBSITE_BASE_URL}/`} />)
}

@tony-sull
Copy link
Contributor

Related RFC discussion: withastro/roadmap#79

@FredKSchott
Copy link
Member

Sort of related: withastro/roadmap#59 (comment)

@tony-sull tony-sull added needs discussion Issue needs to be discussed and removed bb:investigate labels May 10, 2022
@FredKSchott
Copy link
Member

Discussed with some core maintainers, and adding a <base> tag automatically to all 404.html output built files.

Separately, there may be inconsistencies in how we output URLs. To audit and debug as necessary (we should aim to be all relative, or all absolute, we assumed we were all relative but that may no longer be the case).

@FredKSchott FredKSchott added - P4: important Violate documented behavior or significantly impacts performance (priority) s1-small and removed needs discussion Issue needs to be discussed labels May 12, 2022
@tony-sull
Copy link
Contributor

The CSS imports were actually fixed not too long ago, looks like the only asset that 404's here now is the component script that are dynamically loaded for hydrated components

On known pages they'll still be relative but will handle directory depth. In dev and SSR we could handle this by updating the component import based on the 404 URL's depth, but that wouldn't work in build. Not sure what the fix there would be yet 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants