From 76f1b3fee5a1cb12eca0a67d32bc86d93dea7442 Mon Sep 17 00:00:00 2001
From: Aramis Sennyey <sennyeya@amazon.com>
Date: Mon, 5 Dec 2022 18:42:19 -0500
Subject: [PATCH] Review fixes.

Signed-off-by: Aramis Sennyey <sennyeya@amazon.com>
---
 .changeset/five-trainers-admire.md           |  2 +-
 .changeset/young-chairs-check.md             |  2 +-
 packages/core-app-api/src/app/AppManager.tsx | 52 +++++++++++---------
 3 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/.changeset/five-trainers-admire.md b/.changeset/five-trainers-admire.md
index 6d6a023828..e624fc222a 100644
--- a/.changeset/five-trainers-admire.md
+++ b/.changeset/five-trainers-admire.md
@@ -2,4 +2,4 @@
 '@backstage/cli': patch
 ---
 
-Fix webpack dev server issue where it wasn't serving index.html from correct endpoint on subsequent requests.
+Fix webpack dev server issue where it wasn't serving `index.html` from correct endpoint on subsequent requests.
diff --git a/.changeset/young-chairs-check.md b/.changeset/young-chairs-check.md
index 92426dd5e3..e70073e9df 100644
--- a/.changeset/young-chairs-check.md
+++ b/.changeset/young-chairs-check.md
@@ -2,4 +2,4 @@
 '@backstage/core-app-api': patch
 ---
 
-Apps will now rewrite `app.baseUrl` and `backend.baseUrl` to match `location.origin` when `app.baseUrl` is the same as `backend.baseUrl`. This will help reduce the number of frontend builds you have to do with a specific config.
+Apps will now rewrite the `app.baseUrl` configuration to match the current `location.origin`. The `backend.baseUrl` will also be rewritten in the same way when the `app.baseUrl` and `backend.baseUrl` have matching origins. This will reduce the need for separate frontend builds for different environments.
diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx
index 1387130943..b62b857ddf 100644
--- a/packages/core-app-api/src/app/AppManager.tsx
+++ b/packages/core-app-api/src/app/AppManager.tsx
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Config } from '@backstage/config';
+import { AppConfig, Config } from '@backstage/config';
 import React, {
   ComponentType,
   createContext,
@@ -82,7 +82,6 @@ import { ApiRegistry } from '../apis/system/ApiRegistry';
 import { resolveRouteBindings } from './resolveRouteBindings';
 import { BackstageRouteObject } from '../routing/types';
 import { isReactRouterBeta } from './isReactRouterBeta';
-import { JsonObject } from '@backstage/types';
 
 type CompatiblePlugin =
   | BackstagePlugin
@@ -162,20 +161,36 @@ function useConfigLoader(
   if (config.value?.length) {
     const urlConfigReader = ConfigReader.fromConfigs(config.value);
 
-    const resolveRelativeUrl = (relativeUrl: string) =>
-      new URL(relativeUrl, document.location.origin).href;
-
-    const getRelativeUrl = (fullUrl: string) => {
-      const url = new URL(fullUrl);
-      return fullUrl.replace(url.origin, '');
-    };
-
+    /**
+     * Return the origin of the given URL.
+     * @param url An absolute URL.
+     * @returns The given URL's origin.
+     * @throws If fullUrl is not a correctly formatted absolute URL.
+     */
     const getOrigin = (url: string) => new URL(url).origin;
 
+    /**
+     * Resolve an absolute URL as relative to the current document.
+     * @param fullUrl URL to resolve.
+     * @returns Absolute URL with origin as the current document origin.
+     * @throws If fullUrl is not a correctly formatted absolute URL.
+     */
+    const overrideOrigin = (fullUrl: string) => {
+      return new URL(
+        fullUrl.replace(getOrigin(fullUrl), ''),
+        document.location.origin,
+      ).href;
+    };
+
+    /**
+     * Test configs may not define `app.baseUrl` or `backend.baseUrl` and we
+     *  don't want to enforce here.
+     */
     const appBaseUrl = urlConfigReader.getOptionalString('app.baseUrl');
     const backendBaseUrl = urlConfigReader.getOptionalString('backend.baseUrl');
+
     let configs = config.value;
-    const relativeResolverConfig: { data: JsonObject; context: string } = {
+    const relativeResolverConfig: AppConfig = {
       data: {},
       context: 'relative-resolver',
     };
@@ -183,26 +198,15 @@ function useConfigLoader(
       const appOrigin = getOrigin(appBaseUrl);
       const backendOrigin = getOrigin(backendBaseUrl);
 
-      /**
-       * We only want to override the URLs with the document origin when the URLs match
-       *  and are defined. We use getOptionalString here to not throw when the app.baseUrl
-       *  and backend.baseUrl are not defined. If they are defined but not well formatted URLs
-       *  the above getRelativeUrl() method will throw.
-       */
       if (appOrigin === backendOrigin) {
         relativeResolverConfig.data.backend = {
-          baseUrl: resolveRelativeUrl(getRelativeUrl(backendBaseUrl)),
+          baseUrl: overrideOrigin(backendBaseUrl),
         };
       }
     }
     if (appBaseUrl) {
-      /**
-       * Rewriting app.baseUrl to the current document should be a no-op. The
-       *  document hosting the app should always be the same url as the app
-       *  references.
-       */
       relativeResolverConfig.data.app = {
-        baseUrl: resolveRelativeUrl(getRelativeUrl(appBaseUrl)),
+        baseUrl: overrideOrigin(appBaseUrl),
       };
     }
     /**