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

Fix: apply base path to client and websocket #1724

Closed
wants to merge 5 commits into from

Conversation

1nVitr0
Copy link

@1nVitr0 1nVitr0 commented Apr 25, 2023

Adresses #385

Although in my tests this seems to work reliably, this is at best a temporary solution. As discussed in #385, this should be replaced by a long-term solution, such as running nuxt programatically or as it's own thread and proxying requests to it.

Feel free to absolutely obliterate it, or keep this open until the long-term solution is finished. I think we should decide on a direction before I begin tackling that one.

Also, this was done outside the development container (Node 16.20, Ubuntu 22.04), so you might want to look over the lockfile.

@@ -368,14 +368,15 @@ export default {
initializeSocket() {
this.socket = this.$nuxtSocket({
name: process.env.NODE_ENV === 'development' ? 'dev' : 'prod',
channel: this.$config.routerBasePath,
Copy link
Author

Choose a reason for hiding this comment

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

This selects the "basePath" for sockets

server/Server.js Outdated
Comment on lines 348 to 350
const replaceUrls = new RegExp(`((?:href|src|url|scope)"?(?:=|:)\\s*["'](?:http://localhost:3333)?)${currentBasePath || "/"}([^"']*["'])`, "g")
const replaceNuxtConfig = new RegExp(`<script>window.__NUXT__=.*?</script>`, "g")
const replaceAssetPaths = new RegExp(`${currentBasePath}/(_nuxt/[^"]*|sw.js)`, "g")
Copy link
Author

Choose a reason for hiding this comment

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

I looked through every single change this made, and they all looked fine. No guarantee though that it will stay this way, nor that it will always catch every path to be updated.

cors: {
origin: '*',
methods: ["GET", "POST"]
}
})
})).of(global.RouterBasePath || "/")
Copy link
Author

Choose a reason for hiding this comment

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

This will always use the base path as namespace

@advplyr
Copy link
Owner

advplyr commented Apr 25, 2023

Awesome, this is working in my quick test. I'll have to do more testing when I have time. I spotted one path for the comic reader that still needs to get updated https://github.com/advplyr/audiobookshelf/blob/master/client/components/readers/ComicReader.vue#L59

Thanks for getting this moving along!

@1nVitr0
Copy link
Author

1nVitr0 commented Apr 26, 2023

The workerUrl should now update as well, it should now also be easier to update other key-value pairs. I'll be on discord tomorrow if you have time to chat about more permanent solutions.

@1nVitr0 1nVitr0 force-pushed the master branch 3 times, most recently from 29cd63a to 667f58d Compare April 28, 2023 14:12
@1nVitr0
Copy link
Author

1nVitr0 commented May 2, 2023

@advplyr I guess we close this? Feel free to do so 😄

@advplyr advplyr closed this Sep 26, 2023
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.

3 participants