-
Notifications
You must be signed in to change notification settings - Fork 16
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
support deployment to subdirectories #40
base: main
Are you sure you want to change the base?
support deployment to subdirectories #40
Conversation
Hey @jelhan 👋 thanks for your interest in this. Unfortunately it's not quite as simple as just moving the rootUrl because of the nature of the "static api" that also gets deployed at the same time. I think I'll open a discussion on #19 to get into more of an understanding of why this is a good feature to think about and we can then figure out the best way forward 👍 |
Can you point me to the parts that are broken? I haven't done much testing yet to be honest but it seem to work on the first try. At least I didn't noticed any issue when clicking around in a deployed app on a subdirectory. I only see a 404 for |
@mansona Any chance to move forward here? You mentioned that it's not that simple as I have done it here. Could you please share your concerns. I'm happy to work on the issues left if you are willing to accept such merge requests. To be honest I'm not sure how much work is left. I'm using guide maker deployed to a subfolder since a few month now. Didn't faced any issue so far. |
hey @jelhan thanks for the ping 👍 So it turns out that I have recently solved this issue with another Empress product called Field Guide 🎉 and one of the issues that the change you are making is being shown in the netlify failed build at the moment. If you take a look you will see that the prember build is unable to find some of the JSON files This has been fixed in ef4/prember#57 5 days ago 😂 🎉 so, I guess I have a few questions that would help me progress this PR: firstly you say that you've been using it for a few months and you haven't noticed any issues 🤔 does that mean that you were not making use of the prember functionality and just relying on the SPA behaviour? Secondly, which template are you using? I have noticed in my Field Guide experiments that some of the paths to icons and images need to also make sure they are using the rootURL and I haven't yet found a perfect solution to that, and I'm curious to know if you have had similar issues? |
Awesome news! 🎉
Yes. As the documentation created with it is internal only and not that much used I wasn't caring about that one. Actually I didn't noticed until now that it should use Prember. But that should be resolved now if I got you right, isn't it?
I'm using the default template. Faced the same issues as you describe. I fixed them on a feature branch. You could find that merge request here: https://github.com/empress/guidemaker-default-template/pull/6/files For configuration I'm currently relying on duplicating the root URL if needed. That might not be the best solution. We might instead prefix |
so.. I just read your argument in empress/guidemaker-default-template#6 about adding I'm also thinking that rootURL should be supported on a Guidemaker level and not something that should be individually supported by each of the templates 🤔 which gets me thinking that we should make Also I am curious what you mean by |
I meant that adding a controller to calculate the root URL might be a breaking change. I don't see an issue by adding this helper.
That would help with having a harmonized ecosystem for sure. On the other hand it might not be needed for custom templates that don't care about subdirectory support. I don't have a strong opinion on that one.
I was especially referring to the If it's not prefixed automatically the configuration and usage in template would look like this: {
rootUrl: '/foo/',
guidemaker: {
logo: '/foo/images/logo.jpg'
}
} <img src={{this.guidemaker.logo}} /> This is what I'm currently having. Alternatively we could prefix {
rootUrl: '/foo/',
guidemaker: {
logo: '/images/logo.jpg'
}
} <img src={{root-url this.guidemaker.logo}} /> But it's not as simple as this cause absolute URLs that include a domain must not be prefixed with root URL. So this would add some complexity. |
I think the most consistent approach will be to make sure that we use the
What do you mean by this? 🤔 are you saying that you might expect people to put something like this in the config: {
rootUrl: '/foo/',
guidemaker: {
logo: 'https://emberjs.com/images/ember-logo.svg'
}
} because I would consider this an error 🤔 and maybe a major part of fully supporting rootURL properly is just putting warnings and linting in place that makes sure that people are doing things correctly 👍 |
Yes. I thought that this is supported. Treating this as not supported will make implementation way easier. 👍 |
I think it might be accidentally supported at the moment 🤔 but we can definitely warn against it for now 👍 |
Would this include images referenced in the markdown as well? ![some image](/images/foo.jpg) And what about HTML embedded in the markdown? Would we support some kind of |
hmm... 🤔 now we're getting into tricky territory 🤔 I think in the case of markdown image links you might be able to get away with relative urls 🤔 but I know this is probably not ideal. On the other hand, you should be able to use Can you try |
I'm sorry that I missed to follow-up here. Was pulled into other topics.
I tried but the helper does not run. |
Closes #19