fix(@angular-devkit/build-angular): don't override base-href in HTML when it's not set in builder

alan-agius4 · clydin · commit 88701115c69c · 2022-06-30T09:07:00.000-04:00
With this change we fix a regression were we set the base-href to `/` when the browser builder `baseHref` option is not set. Closes #23475
diff --git a/packages/angular_devkit/build_angular/src/builders/app-shell/index.ts b/packages/angular_devkit/build_angular/src/builders/app-shell/index.ts
@@ -118,7 +118,7 @@ async function _renderUniversal(
         projectRoot,
         root,
         outputPath,
-        baseHref,
+        baseHref ?? '/',
         browserOptions.ngswConfigPath,
       );
     }
diff --git a/packages/angular_devkit/build_angular/src/builders/browser/index.ts b/packages/angular_devkit/build_angular/src/builders/browser/index.ts
@@ -79,7 +79,7 @@ export type BrowserBuilderOutput = BuilderOutput & {
   outputs: {
     locale?: string;
     path: string;
-    baseHref: string;
+    baseHref?: string;
   }[];
 };
 
@@ -183,8 +183,6 @@ export function buildWebpackBrowser(
       ({ config, projectRoot, projectSourceRoot, i18n, target, cacheOptions }) => {
         const normalizedOptimization = normalizeOptimization(options.optimization);
 
-        const defaultBaseHref = options.baseHref ?? '/';
-
         return runWebpack(config, context, {
           webpackFactory: require('webpack') as typeof webpack,
           logging:
@@ -319,7 +317,7 @@ export function buildWebpackBrowser(
                   for (const [locale, outputPath] of outputPaths.entries()) {
                     try {
                       const { content, warnings, errors } = await indexHtmlGenerator.process({
-                        baseHref: getLocaleBaseHref(i18n, locale) || defaultBaseHref,
+                        baseHref: getLocaleBaseHref(i18n, locale) ?? options.baseHref,
                         // i18nLocale is used when Ivy is disabled
                         lang: locale || undefined,
                         outputPath,
@@ -363,7 +361,7 @@ export function buildWebpackBrowser(
                         projectRoot,
                         context.workspaceRoot,
                         outputPath,
-                        getLocaleBaseHref(i18n, locale) ?? defaultBaseHref,
+                        getLocaleBaseHref(i18n, locale) ?? options.baseHref ?? '/',
                         options.ngswConfigPath,
                       );
                     } catch (error) {
@@ -393,10 +391,10 @@ export function buildWebpackBrowser(
                   [...outputPaths.entries()].map(([locale, path]) => ({
                     locale,
                     path,
-                    baseHref: getLocaleBaseHref(i18n, locale) ?? defaultBaseHref,
+                    baseHref: getLocaleBaseHref(i18n, locale) ?? options.baseHref,
                   }))) || {
                   path: baseOutputPath,
-                  baseHref: defaultBaseHref,
+                  baseHref: options.baseHref,
                 },
               } as BrowserBuilderOutput),
           ),
diff --git a/packages/angular_devkit/build_angular/src/builders/browser/specs/base-href_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/specs/base-href_spec.ts
@@ -39,6 +39,27 @@ describe('Browser Builder base href', () => {
     await run.stop();
   });
 
+  it('should not override base href in HTML when option is not set', async () => {
+    host.writeMultipleFiles({
+      'src/index.html': `
+      <html>
+        <head><base href="."></head>
+        <body></body>
+      </html>
+      `,
+    });
+
+    const run = await architect.scheduleTarget(targetSpec);
+    const output = (await run.result) as BrowserBuilderOutput;
+
+    expect(output.success).toBeTrue();
+    const fileName = join(normalize(output.outputs[0].path), 'index.html');
+    const content = virtualFs.fileBufferToString(await host.read(fileName).toPromise());
+    expect(content).toContain(`<base href=".">`);
+
+    await run.stop();
+  });
+
   it('should insert base href in the the correct position', async () => {
     host.writeMultipleFiles({
       'src/index.html': tags.oneLine`
diff --git a/packages/angular_devkit/build_angular/src/testing/test-utils.ts b/packages/angular_devkit/build_angular/src/testing/test-utils.ts
@@ -83,8 +83,7 @@ export async function browserBuild(
     };
   }
 
-  const [{ path, baseHref }] = output.outputs;
-  expect(baseHref).toBeTruthy();
+  const [{ path }] = output.outputs;
   expect(path).toBeTruthy();
   const outputPath = normalize(path);
 

Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,7 @@ async function _renderUniversal(`
`118`	`118`	`projectRoot,`
`119`	`119`	`root,`
`120`	`120`	`outputPath,`
`121`		`- baseHref,`
	`121`	`+ baseHref ?? '/',`
`122`	`122`	`browserOptions.ngswConfigPath,`
`123`	`123`	`);`
`124`	`124`	`}`
Original file line number	Diff line number	Diff line change
`@@ -83,8 +83,7 @@ export async function browserBuild(`
`83`	`83`	`};`
`84`	`84`	`}`
`85`	`85`
`86`		`- const [{ path, baseHref }] = output.outputs;`
`87`		`- expect(baseHref).toBeTruthy();`
	`86`	`+ const [{ path }] = output.outputs;`
`88`	`87`	`expect(path).toBeTruthy();`
`89`	`88`	`const outputPath = normalize(path);`
`90`	`89`