-
Notifications
You must be signed in to change notification settings - Fork 2
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
Added support for hosting from a subfolder #248
Conversation
✅ Deploy Preview for kaleidoscopic-pudding-244d33 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
b0bc6a9
to
fee1fb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but I would like more reasoning/discussion on the added script as it feels unnecessary.
app/index.html
Outdated
<!-- support hosting from a subpath such as ipfs --> | ||
<script> | ||
const baseElement = document.createElement('base') | ||
baseElement.setAttribute('href', window.location.href.replace(/\/+$/, '') + '/') | ||
document.head.appendChild(baseElement) | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary? Why not just include a <base href='.'>
element in <head>
? Does this not work and you have to include an explicit base path? That feels wrong to me. 🤔
If the document has no elements, then baseURI defaults to location.href
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced location.href
strategy of parsing subpath by using location.pathname
window.location.pathname.substring(0, window.location.pathname.lastIndexOf('/'))
This commit enable support for hosting as /subpath/
, /subpath/
, /subpath/index.html
and appropriately injects only when needed.
app/index.html
Outdated
|
||
<!-- support hosting from a subpath such as ipfs --> | ||
<script> | ||
const subPath = window.location.pathname.substring(0, window.location.pathname.lastIndexOf('/')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'/subfolder'.lastIndexOf('/') === 0
I don't think this will do what you want as it will strip the subfolder
, but we want to keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const subPath = window.location.pathname.substring(0, window.location.pathname.lastIndexOf('/')) | |
const subPath = window.location.pathname.endsWith('index.html') | |
? window.location.pathname.substring(0, window.location.pathname.length - 'index.html'.length) | |
: window.location.pathname |
I think something like this may work? Double check that a base href of '/subfolder' and '/subfolder/' work the same though, as this will not add a trailing slash if it is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
/
paths to relative path./
Resolves #246
There may still be errors in the page before the base href was set but most modern browsers will find the files after the switch; at least on Chrome and Firefox was tested ok