fix(dashboard): Fix port:auto for api requests #3980
Conversation
this fixes a regression introduced in vendure-ecommerce#3919
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRewrote getApiBaseUrl in config-utils.ts to use globalThis.location for deriving port when Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Dashboard UI
participant Fn as getApiBaseUrl
participant Loc as globalThis.location
UI->>Fn: request API base URL (uiConfig)
alt uiConfig.api.port == 'auto'
Fn->>Loc: read port (globalThis.location.port)
alt port present
Loc-->>Fn: "<port>"
Fn-->>UI: return scheme://host:<port>/path
else port empty
Loc-->>Fn: ""
Fn-->>UI: return scheme://host/path
end
else explicit port provided
Fn-->>UI: return scheme://host:<explicitPort>/path
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dashboard/src/lib/utils/config-utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: michaelbromley
Repo: vendure-ecommerce/vendure PR: 3957
File: packages/create/src/create-vendure-app.ts:382-389
Timestamp: 2025-11-11T12:17:44.531Z
Learning: In the Vendure create CLI quick-start mode, the Dashboard is accessed at the same port as the Vendure server (e.g., `http://localhost:${port}/dashboard`), not at a separate port 3000.
🧬 Code graph analysis (1)
packages/dashboard/src/lib/utils/config-utils.ts (1)
packages/dashboard/src/lib/virtual.d.ts (1)
uiConfig(38-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: e2e tests (24.x, mysql)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (22.x, postgres)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: unit tests (24.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: build (20.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: build (22.x)
- GitHub Check: build (24.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
SiebelsTim
force-pushed
the
dashboard-port-auto
branch
from
fc2c97b to
0026ffc
Compare
|
Thanks for the fix! Can you just take a look at the sonar cloud issues and fix? They are very minor. |
|
Sure! Wasn't sure if I'm supposed to migrate to this. I checked caniuse for globalThis and it seems pretty safe to use. |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/dashboard/src/lib/utils/config-utils.ts (1)
3-8: JSDoc matches behavior; consider noting browser-location dependency.The documentation accurately describes what
getApiBaseUrl()does. For future maintainers (and to avoid confusion in any non-DOM/SSR context), you might optionally add a short note that'auto'derives host/port from the current browser location (location), i.e. this utility is intended for the Dashboard runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dashboard/src/lib/utils/config-utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-11T12:17:44.531Z
Learnt from: michaelbromley
Repo: vendure-ecommerce/vendure PR: 3957
File: packages/create/src/create-vendure-app.ts:382-389
Timestamp: 2025-11-11T12:17:44.531Z
Learning: In the Vendure create CLI quick-start mode, the Dashboard is accessed at the same port as the Vendure server (e.g., `http://localhost:${port}/dashboard`), not at a separate port 3000.
Applied to files:
packages/dashboard/src/lib/utils/config-utils.ts
🧬 Code graph analysis (1)
packages/dashboard/src/lib/utils/config-utils.ts (1)
packages/dashboard/src/lib/virtual.d.ts (1)
uiConfig(38-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: e2e tests (24.x, sqljs)
- GitHub Check: e2e tests (24.x, postgres)
- GitHub Check: e2e tests (24.x, mysql)
- GitHub Check: e2e tests (22.x, postgres)
- GitHub Check: e2e tests (22.x, mariadb)
- GitHub Check: e2e tests (22.x, sqljs)
- GitHub Check: e2e tests (24.x, mariadb)
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: build (20.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: unit tests (24.x)
- GitHub Check: build (24.x)
- GitHub Check: build (22.x)
🔇 Additional comments (1)
packages/dashboard/src/lib/utils/config-utils.ts (1)
13-16: Port handling for'auto'correctly respects the current location (including standard ports).Using
globalThis.locationfor the host whenuiConfig.api.host === 'auto'and thelocationPortParthelper to conditionally append:${location.port}cleanly fixes the regression: non-standard ports are now honored, and standard ports (80/443) no longer produce a trailing colon. This aligns with the Dashboard running on the same port as the Vendure server in quick-start setups. Based on learnings, this behavior is exactly what we want.
2 participants
Description
When the API port is configured to be 'auto' (which is the default) and we are using a non-standard (443,80) port, we cannot login as the API client does not honor the port of the currently accessed page.
This is a regression introduced in #3919. The healthcheck endpoint did not respect
window.location.portas it was building a relative URL. The extracted utility is always building an absolute URL and therefore needs to respectwindow.location.Breaking changes
no, fixes regression.
Screenshots
You can add screenshots here if applicable.
Checklist
📌 Always:
👍 Most of the time:
Summary by CodeRabbit
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.