404 Error on /code-of-conduct and on /contribute/access

[HritvikBhatia]

Most appropriate sections of the p5.js website?

About

What is your operating system?

Windows

Web browser and version

Google Chrome 134.0.6998.36 (Official Build) (64-bit) (cohort: Stable)

Actual Behavior

Navigating to http://localhost:4321/code-of-conduct throws a 404 error.
This issue occurs

when running locally (npm run dev or npm run preview), but works correctly on p5js.org.

Expected Behavior

Clicking on the "Code of Conduct" and "Access Statement" link should always load the page correctly

Steps to reproduce

Run npm run dev
Go to about or http://localhost:4321/about/
Click on "Code of Conduct" or "Access Statement" both are not working

Would you like to work on the issue?

I would like to work on this issue

[HritvikBhatia]

The issue is likely due to the trailingSlash: "always" setting in astro.config.mjs. This forces URLs to always have a trailing slash, but some internal links might be missing it

Should we update the trailingSlash setting to "ignore", or would it be better to add a trailing slash to the links for the Code of Conduct and Access Statement?

[ksen0]

@HritvikBhatia Can you try http://localhost:4321/code-of-conduct/ and update here if the problem persists please? Adding the trailing /. On the live website, the trailing slash is always added. On local, sometimes the links are missing it, causing this issue.

[HritvikBhatia]

Thank you for taking the time to look into this! @ksen0

I tested with the trailing slash, and it works when manually added. However, when I click the link, it still opens http://localhost:4321/code-of-conduct (without the trailing slash). Manually adding / to the links fixes the issue. Would it be okay if I make a PR to update the links accordingly?

Also, since issue #723 prevented me from running npm run dev on Windows, I had already submitted PR #724 to fix it. Would it be possible to review and merge it? Otherwise, I have to keep making the same changes locally each time I work on the project.

Thanks again for your time and help! I appreciate it. 😊

[ksen0]

Hi @HritvikBhatia, I've just merged #724 !

Would it be okay if I make a PR to update the links accordingly?

Yes please feel free to add trailing slash, thank you!

PS I saw the all-contributors bot wasn't working with your @. I can either add it manually in a few days, or if you have a minute you can fork p5.js repo, add yourself to https://github.com/processing/p5.js/blob/main/.all-contributorsrc and then all-contributors generate in command line will update the README.md. So if you make a PR in p5.js repository with those 2 files updated to include yourself, you can @ me and I'll merge, not sure why the all-contributor bot is not working. (If you figure out what's up with the bot, even better, I haven't seen it do that before.)

[HritvikBhatia]

Initially, the focus was on fixing the "Code of Conduct" and "Access Statement" links, but while reviewing, I found and fixed several other broken links that also needed a trailing slashes.

Thanks, @ksen0! I’ll update the contributors list, and submit a PR soon. Thanks for trying to look into it! 😊

[ksen0]

@HritvikBhatia Thank you! I can imagine others would also have the same problem.

Regarding your original comment, just to address it in a little more detail than I did before:

The issue is likely due to the trailingSlash: "always" setting in astro.config.mjs. This forces URLs to always have a trailing slash, but some internal links might be missing it

I think the bigger issue here is that the behavior of the production environment differs from the development environment. I am not sure whether the proposed config change would address this issue, maybe actually it does; however, if the underlying issue is not addressed, then I feel more comfortable merging changes with minimal possible side effects. For example, if the trailing slash config is updated, maybe that could impact on some redirect rules somewhere, and because I myself am pretty new to the project I am not that confident that I know all possible places that might cause side-effects or how to verify it's all fine. Your PR adding trailing slashes to links is much easier to verify that that it is effective and has no side-effects, so it's easier to get it merged.

[HritvikBhatia]

Thanks for the detailed explanation That makes sense, and I’m okay with any approach that works best for the project. Let me know if anything else needs to be adjusted

[davepagurek]

I think the bigger issue here is that the behavior of the production environment differs from the development environment. I am not sure whether the proposed config change would address this issue, maybe actually it does; however, if the underlying issue is not addressed, then I feel more comfortable merging changes with minimal possible side effects. For example, if the trailing slash config is updated, maybe that could impact on some redirect rules somewhere, and because I myself am pretty new to the project I am not that confident that I know all possible places that might cause side-effects or how to verify it's all fine. Your PR adding trailing slashes to links is much easier to verify that that it is effective and has no side-effects, so it's easier to get it merged.

Hey everyone! To add some context: we used to have a bunch of issues where some code would only work locally and some code would only work on the live site because the behaviour of relative links is different depending on whether or not there is a trailing slash.

For example, if you are on /tutorials and you click on <a href="something">, you end up on /something. If you were on /tutorials/ to start, you would end up on /tutorials/something. Since the live site never lets you go on /tutorials (it always adds the trailing slash), allowing you to go to /tutorials locally would open up the possibility of accidentally introducing that kind of bug again (e.g. linking to tutorials/something, which would navigate to /tutorials/something just fine locally, but will land you on /tutorials/tutorials/something on the live site.)

So even though the more permissive trailing slash setting would make some broken links work, I figured it would be best to keep those links broken locally given that they work on the live site, and we can just incrementally fix them when we find them. Which is exactly what you've helped us do @HritvikBhatia, thanks!

[davepagurek]

Ideally, there would be a way to make the behaviour the same in both contexts. Since I don't think we can control the github pages deployment, a possible long-term fix could be to find a way on the Astro dev server to redirect to add a trailing slash? I think I looked briefly and didn't find a way, but that was a while ago so things may have changed or I may have missed something.

(Also @ksen0, if we're deploying a 2.0 site using something other than github pages, we might want to experiment and see if going to /tutorials will auto-redirect to /tutorials/. If not, we might end up with some broken links on that deployment that need fixing.)

[ksen0]

@davepagurek This slipped through, but I just tested that the Netlify deployment does also add the trailing slashes in 2.0 deployment. I am not sure I 100% follow the explanation above but as far as I understand, adding trailing slashes one case by case basis has no side effects, right? If it does please feel free to re-open1

[davepagurek]

Right, adding trailing slashes will keep our production deploys working the same, and slowly make dev servers work better. We can always open some issues for the known spots that need them (the bottom left sidebar showing related reference entries, for example.)