feat(connect-ui): support non-root base path#6765
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Self-hosters behind a reverse proxy that routes by path (everything under a shared prefix, no per-service subdomain) could not serve Connect UI: the bundle was built with a root-relative base, so assets resolved at the origin root and the app failed to load. Build the bundle with a placeholder base and rewrite it to the configured path at container start (derived from NANGO_PUBLIC_CONNECT_URL, or an explicit NANGO_CONNECT_UI_BASE_PATH override; defaults to "/"). The prebuilt image needs no rebuild. The router basepath is read from import.meta.env.BASE_URL so client-side routing works under the sub-path.
Address review feedback on base path handling: - Insert the base path with a function replacer so a literal `$` in it isn't read as a replacement pattern ($&, $1, ...). - Strip any query or fragment mistakenly passed in the explicit NANGO_CONNECT_UI_BASE_PATH override. - Extract the BASE_URL -> router basepath derivation into a pure function and cover it, resolveBasePath, and the query/fragment case with tests.
The connect_ui deploy workflow builds the bundle and syncs it straight to static hosting (e.g. connect.nango.dev). Since the build now emits a placeholder base, that upload would ship unresolved placeholders and break the hosted UI. Run set-base-path.js after the build so the placeholder is rewritten to "/" (the origin root) before upload. Also clarify the surrounding comments: the rewrite is mandatory before serving (not a no-op for root), and note the docs/self-hoster follow-up.
Document serving Connect UI under a non-root base path via NANGO_PUBLIC_CONNECT_URL (with the NANGO_CONNECT_UI_BASE_PATH override) and the reverse-proxy prefix-stripping requirement.
Expose the base-path rewrite as `npm run -w @nangohq/connect-ui set-base-path` and use it in the entrypoint and the connect_ui deploy workflow. Gives self-hosters who serve the static bundle themselves an ergonomic command instead of a raw node invocation.
Self-hosters serving Connect UI's static files from their own host must run the base-path rewrite before uploading; the built assets carry a placeholder base until then.
The base path is written verbatim into the built HTML/CSS/JS, so a malformed value could emit broken or unsafe asset URLs. Reject anything outside a safe URL-path character set so misconfiguration fails loudly at rewrite time instead. Also guards the JS template-literal context, where a "$" or backtick in the base could otherwise break out of the string.
Run the base-path rewrite as the first step of serve:unsafe so any deployment that serves the static bundle with that command works — not just the FLAG_SERVE_CONNECT_UI entrypoint. This covers self-hosters who run Connect UI as a standalone container (command = serve:unsafe) without the server entrypoint, and removes the manual pre-step for them. The entrypoint no longer needs a separate set-base-path call.
The rewrite runs for both FLAG_SERVE_CONNECT_UI and a standalone serve:unsafe container; the manual step is only for uploading dist to a static host/CDN.
The connect_link returned by POST /connect/sessions and .../reconnect is the shareable "open Connect UI" link. It was built as connectUrl plus the session token, ignoring any base path, so a self-hoster serving Connect UI under a sub-path got a broken link even when the embedded iframe worked. Build it via buildConnectUiSessionLink, which applies the base path from NANGO_PUBLIC_CONNECT_URL (or NANGO_CONNECT_UI_BASE_PATH).
The rewrite kept a pristine .dist-template copy so it could be re-run with a different base path in place. Every deployment starts from a freshly built dist (ephemeral containers, deploy builds fresh), and the base path is fixed per container via env vars, so that never happens. Simplify to a one-shot placeholder replace. Also move the pure resolveBasePath tests to a node test and split vitest into browser and node projects so the script has real coverage.
Validate NANGO_CONNECT_UI_BASE_PATH in the ENVS schema so a malformed value fails at startup instead of throwing when a connect session is created. buildConnectUiSessionLink now takes the connect URL and base path as arguments, and the controllers pass the server's validated envs rather than the util reading process.env. Also cross-reference the base-path contract between the util and the connect-ui rewrite script, which can't share code across the package boundary.
Extract the base-path source (override → connect URL path → root) into a small helper with early returns instead of reassigning a `raw` variable, and drop the TODO comment (tracked on the PR/ticket instead).
The browser project's include already scopes to src/**/*.test.tsx, but exclude the node test glob explicitly (preserving vitest's defaults) so a node script test can't be pulled into the chromium runner.
e09f1a0 to
63a32c7
Compare
Deriving the base path from NANGO_PUBLIC_CONNECT_URL silently fell back to root when the URL was set but unparseable, masking a misconfiguration. Throw instead so it fails loudly at startup, consistent with the base path character validation.
Drop NANGO_CONNECT_UI_BASE_PATH. The base path is by definition the path of NANGO_PUBLIC_CONNECT_URL — the frontend SDK loads the Connect UI iframe from that URL, so it must already include any sub-path. A separate override could only duplicate or drift from it. This also simplifies the session link: since the public URL already includes the sub-path, buildConnectUiSessionLink just appends the session token (the base-path normalization it did was only needed for the override), and the controllers pass the validated envs value.
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/guides/platform/self-hosting.mdx">
<violation number="1" location="docs/guides/platform/self-hosting.mdx:219">
P2: Standalone and `FLAG_SERVE_CONNECT_UI` deployments on a read-only filesystem fail before serving because startup rewrites files in `dist`. Mention that the Connect UI `dist` directory must be writable when using this automatic rewrite.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The startup rewrite edits files in dist in place, so a read-only root filesystem would fail before serving.
Keep it, but out of the rewrite paragraph so it's easier to spot.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The session link is just NANGO_PUBLIC_CONNECT_URL + the session token,
and that URL already includes any sub-path, so master's inline
`new URL(`${connectUrl}?session_token=...`)` was already correct. Remove
the buildConnectUiSessionLink util and revert the two controllers.
A read-only root filesystem still works if dist is a writable volume or tmpfs; only dist needs to be writable.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
An empty tmpfs/volume mounted over dist shadows the image's prebuilt assets, so the rewrite finds nothing. State the requirement (dist must be writable at runtime) without prescribing a workaround that breaks.
TBonnin
left a comment
There was a problem hiding this comment.
Can we use the same pattern the webapp already uses for /env.js and window._env?
I mean building ConnectUI with relative asset paths (base: './' in vite.config.ts) so assets just resolve against wherever index.html was served from, and have the router learn its base path from a tiny /connect-env.js script computed fresh on every request from NANGO_PUBLIC_CONNECT_URL.
We could avoid requiring dist to be writable at runtime and changing NANGO_PUBLIC_CONNECT_URL would takes effect immediately on restart.
|
Ah great idea, I couldn't think of a good non-invasive solution myself but this makes sense to me. I've drafted the solution in a separate PR #6802 The main complexity seems to be in trailing slash handling as the connect ui with relative base path doesn't work otherwise. But seems like a better approach than what's in this PR with fewer limitations/workarounds. Let me know what you think about that one 🙏 |
Problem
Connect UI's built assets are referenced from the domain root (
/assets/…), which only works when it's served at its own origin (likeconnect.nango.dev). Self-hosters who can't use a subdomain serve it under a shared path prefix (e.g.example.com/nango/connect/) — there the browser still requests assets at the root, they 404, and the app never loads.Solution
Build with a placeholder base and rewrite it to the real base path before serving, so the prebuilt image needs no rebuild.
serve:unsafe(covering theFLAG_SERVE_CONNECT_UIentrypoint and standalone containers); theconnect_uideploy runs it before upload.NANGO_PUBLIC_CONNECT_URL(the SDK loads the iframe from that URL, so it already includes any sub-path), defaulting to/. No separate config knob.import.meta.env.BASE_URLso client-side routing works under the sub-path.Fixes NAN-6242
Testing
connect_uito dev from this branch:connect-development.nango.devand launching Connect UI fromapp-development.nango.devboth work.resolveBasePathunit tests (root, sub-path, override, malformed URL, unsafe characters).Smartsheet customer
Resolves their reported issue: standalone
serve:unsafecontainer behind a prefix-stripping proxy, served under/3.0/connectors/connect_ui/. They setNANGO_PUBLIC_CONNECT_URLto that path — no command change. To confirm with them: writable container filesystem, and the proxy strips the prefix.Breaking changes
Connect UI's built assets now ship with a placeholder base that must be rewritten before serving. The rewrite runs automatically in
serve:unsafe(theFLAG_SERVE_CONNECT_UIentrypoint and standalone containers) and in theconnect_uideploy. Root deployments on a writable filesystem — the default — are unaffected.Two configurations break and need action:
distrequired: the rewrite editsdistin place at startup, so that directory must be writable at runtime.distto a static host/CDN yourself instead of serving it via the container, runnpm run -w @nangohq/connect-ui set-base-path(withNANGO_PUBLIC_CONNECT_URLset) againstdistbefore uploading — the assets carry a placeholder base until then.This should be called out in the next managed image release notes.
Follow-ups (not in this PR):
docs/updates/changelog.mdxentry when the managed image release is cut.distfrom their own static host (would need the rewrite step before upgrading).