-
-
Notifications
You must be signed in to change notification settings - Fork 389
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
feat: add configurable base URL #1359
Conversation
0dd82a3
to
fe2a11d
Compare
this is now working on my machine, so it should be good to review. the only thing not checked is the github actions changes (also, it's up to the maintainers whether you want to have a second build or not, it would impact your project's storage usage slightly) |
0d8bb72
to
432605f
Compare
note: this addresses all the issues in my own setup, but there's probably other paths I've missed. I'm not a web developer so this was all pulled together through trial and error, there should probably be a test case added to catch any regressions / missed cases |
Thx for your PR. This PR with the changed build workflow would "kill" all default installations. So the default should not be changed. I have to check the rest on my test rig. |
I would prefer a dynamic solution (editable via config.json file). I see a separate build for subdirectories as a workaround. I couldn't test the build yet, but I wouldn't be sure now if the API would also run via reverse proxy. |
that would be ideal, unfortunately a lot of the static assets are compiled at build time that contain absolute paths, it's possible some of them could be changed to relative paths but with the virtual multi-page system I'm not sure how that would interact. I don't love this solution either but I didn't see any other approach to take that wouldn't involve major architectural changes. If you don't want to publish multiple builds, I can remove the github actions changes, I think just upstreaming this build option would already be a huge win for weird nerds like myself who want to serve each webapp at its own subpath. I'd be happy to write some docs for this feature as well, let me know where to look and if that's in-repo I can put it in this PR. As for moonraker, this PR shouldn't change anything about where it looks for moonraker ( |
Are you able to use the Moonraker update manager to use the subpath? |
i don't think so, looks like that would require adding functionality to Moonraker to set build-time environment variables |
nono... moonraker is not building mainsail. it just download the prebuild zip from the latest release. |
oh right, my bad. taking a look at the update process, looks like right now it's hardcoded to just use the first asset associated with a Release: So unless that's updated to iterate through the release assets with some name matching logic exposed as a config option, I should remove the github actions part from this PR. I'm busy with work for the next while but I can try to follow up on this later this week, either creating a moonraker PR to add that logic to its updater or just removing the release part of this PR if you'd rather defer that for now |
68111c4
to
35bddc5
Compare
this is a base env file, it's ok to include this in the repo. The one that should be excluded is .env.local, and that's already covered by the *.local line Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
Signed-off-by: Alice <[email protected]>
This reverts commit b261075. Signed-off-by: Alice <[email protected]>
Sorry for the delay. I've reverted the commit that adds the github builds, that should prevent any issues with Moonraker's auto-updater accidentally grabbing the wrong build. I've also rebased to clear up some merge commits, and it looks like #1336 made a lot of this PR's changes redundant, so the diff should look a lot smaller now |
looks like I introduced an error, I'll un-draft this once that's fixed |
Signed-off-by: Alice <[email protected]>
i will close this PR, because of no response... Please open a new PR, if you have an idea how to fix this issue. |
totally understandable, I haven't had time to work on this, and it's sat too long by now |
This PR will fix #1163
Adds a
VITE_BASE
environment variable at build-time that allows the user to serve the app from non-root URL paths.Signed-off-by: Alice [email protected]