Skip to content

fix(@angular/build): allow allowed-hosts CLI option for dev-server - #34595

Closed
ant1quar wants to merge 1 commit into
angular:mainfrom
ant1quar:fix/dev-server-allowed-hosts-cli
Closed

ant1quar wants to merge 1 commit into
angular:mainfrom
ant1quar:fix/dev-server-allowed-hosts-cli

Conversation

@ant1quar

Copy link
Copy Markdown

Summary

  • Change @angular/build dev-server allowedHosts schema to a string array (matching @angular-devkit/build-angular) so the CLI parses --allowed-hosts example.com correctly.
  • Add disableHostCheck and map it to Vite allowedHosts: true during option normalization.
  • Add dev-server and CLI json-schema tests for the new behavior.

Fixes #33955

Test plan

  • pnpm bazel test //packages/angular/cli:test --test_filter="*dev-server schema*"
  • pnpm bazel test //packages/angular/build:dev-server_integration_tests --test_filter="*allowedHosts*|*disableHostCheck*"

Made with Cursor

@google-cla

google-cla Bot commented Oct 29, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a 'disableHostCheck' option to the dev-server builder, updates the schema to support this change, and adds relevant unit tests. The review feedback highlights opportunities to simplify type definitions by removing redundant assertions and Omit operations, as well as recommendations to improve the robustness of file path resolution in tests by using 'import.meta.url' instead of 'process.cwd()'.

purgeStaleBuildCache,
} from './internal';
import { normalizeOptions } from './options';
import { normalizeOptions, type NormalizedDevServerOptions } from './options';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since the redundant type assertion has been removed, we no longer need to import NormalizedDevServerOptions in this file.

Suggested change
import { normalizeOptions, type NormalizedDevServerOptions } from './options';
import { normalizeOptions } from './options';

Comment on lines +78 to +82
const normalizedOptions = (await normalizeOptions(
context,
projectName,
initialOptions,
)) as NormalizedDevServerOptions;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type assertion as NormalizedDevServerOptions is redundant here. The return type of normalizeOptions is already fully compatible with NormalizedDevServerOptions (since strictPort: boolean is assignable to strictPort?: boolean and allowedHosts matches exactly). Removing this assertion simplifies the code and avoids potential future type-safety issues if the types diverge.

Suggested change
const normalizedOptions = (await normalizeOptions(
context,
projectName,
initialOptions,
)) as NormalizedDevServerOptions;
const normalizedOptions = await normalizeOptions(
context,
projectName,
initialOptions,
);

Comment on lines 16 to 22
export type NormalizedDevServerOptions = Omit<
Awaited<ReturnType<typeof normalizeOptions>>,
'strictPort'
'strictPort' | 'allowedHosts'
> & {
strictPort?: boolean;
allowedHosts: true | string[];
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Omitting and re-adding allowedHosts in NormalizedDevServerOptions is redundant because normalizeOptions already returns allowedHosts with the type true | string[]. We can simplify this definition by only omitting and overriding strictPort.

export type NormalizedDevServerOptions = Omit<
  Awaited<ReturnType<typeof normalizeOptions>>,
  'strictPort'
> & {
  strictPort?: boolean;
};

Comment on lines +10 to +11
import { readFileSync } from 'node:fs';
import path from 'node:path';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Import fileURLToPath from node:url to resolve the schema path robustly using import.meta.url.

Suggested change
import { readFileSync } from 'node:fs';
import path from 'node:path';
import { readFileSync } from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';

Comment on lines +396 to +399
const devServerSchemaPath = path.join(
process.cwd(),
'../build/src/builders/dev-server/schema.json',
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using process.cwd() to resolve the schema path is fragile because it depends on the current working directory of the test runner (which can vary between Bazel, local IDE test runners, or running from the workspace root). Using import.meta.url with fileURLToPath is a robust, location-independent way to resolve paths relative to the test file.

    const devServerSchemaPath = fileURLToPath(
      new URL('../../../../build/src/builders/dev-server/schema.json', import.meta.url),
    );

@ant1quar
ant1quar force-pushed the fix/dev-server-allowed-hosts-cli branch 4 times, most recently from eeec444 to 93fcd18 Compare October 29, 2026 22:05
@ant1quar
ant1quar force-pushed the fix/dev-server-allowed-hosts-cli branch from 93fcd18 to eae16aa Compare October 29, 2026 22:10
@alan-agius4

Copy link
Copy Markdown
Collaborator

Hi @ant1quar,

Thank you for taking the time to contribute to the Angular CLI!

Unfortunately, modifying the allowedHosts schema to strictly be an array and removing boolean support is a breaking change for existing projects that currently configure allowedHosts: boolean (such as allowedHosts: true) in their angular.json.

Because breaking schema changes cannot be accepted at this time in a minor release, we cannot accept this pull request as proposed.

Thank you again for your efforts and understanding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't use an array of hostnames in --allowedHosts cli parameter in @angular/build:dev-server

2 participants