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

WIP - Force trailing slash #352

Closed
wants to merge 2 commits into from

Conversation

josephearl
Copy link
Contributor

@josephearl josephearl commented Apr 11, 2017

- Summary

Attempt at fixing #332 with React Router, does not work currently. Needs further investigation.

- Description for the changelog

If the path name does not end in a single trailing slash when the app is loaded redirect to a URL with a single trailing slash before loading the rest of the app.

src/index.js Outdated
document.body.appendChild(el);
const fixUrl = (pathname) => {
window.location.href =
`${ window.location.protocol }//${ window.location.host }${ pathname }`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: we can shrink this down by first destructuring window.location.

src/index.js Outdated
const el = document.createElement('div');
el.id = 'root';
document.body.appendChild(el);
const fixUrl = (pathname) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with your mention of placement - it would be good to put this somewhere route specific, and try to involve React Router if possible. I like this approach. We could then put the rewriting functions in a separate file under src/routing. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will take a look at the react router solution you linked and see if I can do it that way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave this a go (see new commit) -- but unfortunately it didn't work - if you have a URL http://localhost/admin, http://localhost/admin/ or http://localhost/admin// the path is / as far React router is concerned.

@erquhart erquhart changed the title Force trailing slash WIP - Force trailing slash Apr 13, 2017
@josephearl
Copy link
Contributor Author

josephearl commented Apr 14, 2017

@erquhart thanks for the title update - I had been meaning to do that!
Updated the description.

@erquhart
Copy link
Contributor

erquhart commented Sep 2, 2017

Closing this - considering that we don't know where folks will put the CMS and how their build system will interact with it, we'll need a more considered approach to solve this issue (if at all).

@erquhart erquhart closed this Sep 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants