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

Add ending / to ensure the path-relative resources are referenced correctly #20

Merged
merged 6 commits into from
Nov 29, 2022
Merged

Add ending / to ensure the path-relative resources are referenced correctly #20

merged 6 commits into from
Nov 29, 2022

Conversation

JakeChampion
Copy link
Contributor

Without this change, the path-relative resources (/assets/...) attempt to get loaded from the root path, which is not where they exist and so a 404 is returned.

This patch corrects the redirected location to ensure that it ends in / as then the path-relative resources will load from /stable/assets or /dev/assets, which is where they exist

citkane and others added 6 commits August 23, 2022 17:02
Fix documentation root and failing node v14 15 tests
…rectly

Without this change, the path-relative resources (/assets/...) attempt to get loaded from the root path, which is not where they exist and so a 404 is returned.

This patch corrects the redirected location to ensure that it ends in `/` as then the path-relative resources will load from `/stable/assets` or `/dev/assets`, which is where they exist
@JakeChampion JakeChampion marked this pull request as ready for review November 24, 2022 15:39
@toebeann
Copy link
Collaborator

In which browsers does this issue manifest? All modern browsers should automatically redirect from /dev to /dev/ and so on. Would like to test this reported behaviour.

Does the issue described affect our site at https://citkane.github.io/typedoc-plugin-versions ?

@JakeChampion
Copy link
Contributor Author

@toebeann This issue is in all browsers for me, could you try this site? https://formally-flexible-anemone.edgecompute.app

@toebeann
Copy link
Collaborator

Ah, this is likely a difference of server configuration.

I suppose it does likely make the most sense for us to add this trailing slash to mitigate inconsistencies between servers.

Could you please retarget this PR against the dev branch @JakeChampion ?

@JakeChampion JakeChampion changed the base branch from main to dev November 29, 2022 13:41
@JakeChampion
Copy link
Contributor Author

@toebeann thanks - I have re-targeted it now

@toebeann toebeann self-assigned this Nov 29, 2022
@toebeann
Copy link
Collaborator

Unfortunately our GitHub test workflow has missed this - supposedly because it wasn't initially targeting 'dev' when created. Closing and re-opening as it should be triggered on 'reopened' 🤞

@toebeann toebeann closed this Nov 29, 2022
@toebeann toebeann reopened this Nov 29, 2022
@toebeann
Copy link
Collaborator

There she goes

@toebeann
Copy link
Collaborator

Boom, green light across the board. Thanks for the contribution @JakeChampion. Short of any complications, I'll get this merged down and released to npm promptly 🚀

@toebeann toebeann merged commit a385d8c into citkane:dev Nov 29, 2022
@toebeann toebeann mentioned this pull request Nov 29, 2022
@JakeChampion JakeChampion deleted the patch-1 branch November 29, 2022 14:16
@toebeann toebeann added the bug Something isn't working label Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants