-
Notifications
You must be signed in to change notification settings - Fork 319
feat: add subpath config #1236
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 subpath config #1236
Conversation
🦋 Changeset detectedLatest commit: 3181f27 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@datnguyennnx is attempting to deploy a commit to the HyperDX Team on Vercel. A member of the Team first needs to authorize it. |
packages/api/.env.development
Outdated
HYPERDX_LOG_LEVEL=debug | ||
EXPRESS_SESSION_SECRET="hyperdx is cool 👋" | ||
FRONTEND_URL="http://localhost:${HYPERDX_APP_PORT}" | ||
FRONTEND_URL="http://localhost:4040/hyperdx" |
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.
we don't need to update this unless devs want to test out the proxy
packages/app/next.config.js
Outdated
}); | ||
|
||
module.exports = { | ||
basePath: process.env.HYPERDX_BASE_PATH || '', |
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.
Let's call it NEXT_PUBLIC_BASE_PATH
. We also need to verify if this works on production build
.env
Outdated
HYPERDX_APP_URL=http://localhost | ||
HYPERDX_LOG_LEVEL=debug | ||
HYPERDX_OPAMP_PORT=4320 | ||
HYPERDX_BASE_PATH=/hyperdx |
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.
Let's roll this back since it affects prod
@@ -0,0 +1,35 @@ | |||
http: |
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.
Can you add a README file to proxy
directory to explain what it’s about? It would also be helpful to mention NEXT_PUBLIC_BASE_PATH
and FRONTEND_URL
env vars
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@datnguyennnx Let me know if you’ve had a chance to review those comments. We’d love to get this PR merged this week. |
Apologies for the delay in my response. @wrn14897 I've now updated the configurations for both NGINX and Traefik to handle the subpath routing at the proxy layer, aligning with the architectural direction we discussed. I also updated the proxy/README.md to reflect these changes and provide clear instructions. |
Theoretically one env var should be sufficient but its okay to keep this structure for now. Thanks for the contribution! |
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.
LGTM
Introduce the new flag NEXT_PUBLIC_BASEPATH to simplify subpath configuration for HyperDX deployments. Builds on the flexible basePath config added in [PR #1188], making it easier to handle frontend routing under custom subpaths using an environment variable. Proxy configuration examples for nginx and Traefik will also be provided for reference.