Skip to content

Commit b6c74ea

Browse files
committed
fix(@schematics/angular): don't show server routing prompt when using browser builder
The new routing APIs don't support `browser` builder, but calling `ng add @angular/ssr` with a `browser` builder would still prompt the user to add them. If the user said "Yes", it would actually ignore that answer and not enable the new APIs. With this change, `ng add @angular/ssr` when using `browser` builder does not show the prompt and assumes the answer is "No". It also throws an error if the user runs `ng add @angular/ssr --server-routing`. I'm not aware of a built-in prompting mechanism in schematics beyond `x-prompt`, which can't be used here, so instead I just called Inquirer directly. Unfortunately testing the prompt is a little awkward, as Inquirier does not provide useful APIs in this space. I evaluated `@inquirer/testing`, but ultimately decided that was more intended for testing custom Inquirer prompts, not mocking usage of standard prompts. Schematics APIs do not provide a useful way to inject additional data like a mock, so instead I had to do this through a `setPrompterForTestOnly` function. I'm not a huge fan of it, but I don't see a more straightforward way of solving the problem.
1 parent 4fcce7d commit b6c74ea

File tree

5 files changed

+127
-8
lines changed

5 files changed

+127
-8
lines changed

packages/schematics/angular/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ ts_library(
8484
"//packages/angular_devkit/schematics",
8585
"//packages/angular_devkit/schematics/tasks",
8686
"//packages/schematics/angular/third_party/github.com/Microsoft/TypeScript",
87+
"@npm//@inquirer/prompts",
8788
"@npm//@types/node",
8889
"@npm//browserslist",
8990
"@npm//jsonc-parser",

packages/schematics/angular/application/index_spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ describe('Application Schematic', () => {
3232
const defaultOptions: ApplicationOptions = {
3333
name: 'foo',
3434
skipPackageJson: false,
35+
serverRouting: false,
3536
};
3637

3738
let workspaceTree: UnitTestTree;

packages/schematics/angular/ssr/index.ts

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ async function getApplicationBuilderOutputPaths(
8585
const { outputPath } = architectTarget.options;
8686
if (outputPath === null || outputPath === undefined) {
8787
throw new SchematicsException(
88-
`outputPath for ${projectName} ${target} target is undeined or null.`,
88+
`outputPath for ${projectName} ${target} target is undefined or null.`,
8989
);
9090
}
9191

@@ -361,19 +361,20 @@ function addServerFile(
361361
};
362362
}
363363

364-
export default function (options: SSROptions): Rule {
364+
export default function (inputOptions: SSROptions): Rule {
365365
return async (host, context) => {
366-
const browserEntryPoint = await getMainFilePath(host, options.project);
366+
const browserEntryPoint = await getMainFilePath(host, inputOptions.project);
367367
const isStandalone = isStandaloneApp(host, browserEntryPoint);
368368

369369
const workspace = await getWorkspace(host);
370-
const clientProject = workspace.projects.get(options.project);
370+
const clientProject = workspace.projects.get(inputOptions.project);
371371
if (!clientProject) {
372372
throw targetBuildNotFoundError();
373373
}
374374

375375
const isUsingApplicationBuilder = usingApplicationBuilder(clientProject);
376-
376+
const serverRouting = await isServerRoutingEnabled(isUsingApplicationBuilder, inputOptions);
377+
const options = { ...inputOptions, serverRouting };
377378
const sourceRoot = clientProject.sourceRoot ?? posix.join(clientProject.root, 'src');
378379

379380
return chain([
@@ -404,3 +405,44 @@ function usingApplicationBuilder(project: ProjectDefinition) {
404405

405406
return isUsingApplicationBuilder;
406407
}
408+
409+
// Wrap inquirer in a `prompt` function.
410+
export type Prompt = (message: string, defaultValue: boolean) => Promise<boolean>;
411+
const defaultPrompter: Prompt = async (message, defaultValue) => {
412+
const { confirm } = await import('@inquirer/prompts');
413+
return await confirm({
414+
message,
415+
default: defaultValue,
416+
});
417+
};
418+
419+
// Allow the prompt functionality to be overridden to facilitate testing.
420+
let prompt = defaultPrompter;
421+
export function setPrompterForTestOnly(prompter?: Prompt): void {
422+
prompt = prompter ?? defaultPrompter;
423+
}
424+
425+
/** Returns whether or not server routing is enabled, potentially prompting the user if necessary. */
426+
async function isServerRoutingEnabled(
427+
isUsingApplicationBuilder: boolean,
428+
options: SSROptions,
429+
): Promise<boolean> {
430+
if (!isUsingApplicationBuilder) {
431+
if (options.serverRouting) {
432+
throw new SchematicsException(
433+
'Server routing APIs can only be added to a project using `application` builder.',
434+
);
435+
} else {
436+
return false;
437+
}
438+
}
439+
440+
// Use explicit option if provided.
441+
if (options.serverRouting !== undefined) return options.serverRouting;
442+
443+
// Prompt the user if no option was provided.
444+
return await prompt(
445+
'Would you like to use the Server Routing and App Engine APIs (Developer Preview) for this server application?',
446+
/* defaultValue */ false,
447+
);
448+
}

packages/schematics/angular/ssr/index_spec.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/te
1010

1111
import { join } from 'node:path';
1212
import { Schema as ServerOptions } from './schema';
13+
import { Prompt, setPrompterForTestOnly } from './index';
1314

1415
describe('SSR Schematic', () => {
1516
const defaultOptions: ServerOptions = {
1617
project: 'test-app',
18+
serverRouting: false,
1719
};
1820

1921
const schematicRunner = new SchematicTestRunner(
@@ -30,6 +32,10 @@ describe('SSR Schematic', () => {
3032
};
3133

3234
beforeEach(async () => {
35+
setPrompterForTestOnly((message) => {
36+
return fail(`Unmocked prompt: ${message}`) as never;
37+
});
38+
3339
appTree = await schematicRunner.runExternalSchematic(
3440
'@schematics/angular',
3541
'workspace',
@@ -150,6 +156,56 @@ describe('SSR Schematic', () => {
150156
server: 'node-server',
151157
});
152158
});
159+
160+
it('generates server routing configuration when enabled', async () => {
161+
const tree = await schematicRunner.runSchematic(
162+
'ssr',
163+
{ ...defaultOptions, serverRouting: true },
164+
appTree,
165+
);
166+
167+
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeTrue();
168+
});
169+
170+
it('does not generate server routing configuration when disabled', async () => {
171+
const tree = await schematicRunner.runSchematic(
172+
'ssr',
173+
{ ...defaultOptions, serverRouting: false },
174+
appTree,
175+
);
176+
177+
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeFalse();
178+
});
179+
180+
it('generates server routing configuration when prompt is accepted by the user', async () => {
181+
const prompter = jasmine.createSpy<Prompt>('prompt').and.callFake(async () => true);
182+
setPrompterForTestOnly(prompter);
183+
184+
const tree = await schematicRunner.runSchematic(
185+
'ssr',
186+
{ ...defaultOptions, serverRouting: undefined },
187+
appTree,
188+
);
189+
190+
expect(prompter).toHaveBeenCalledTimes(1);
191+
192+
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeTrue();
193+
});
194+
195+
it('does not generate server routing configuration when prompt is rejected by the user', async () => {
196+
const prompter = jasmine.createSpy<Prompt>('prompt').and.callFake(async () => false);
197+
setPrompterForTestOnly(prompter);
198+
199+
const tree = await schematicRunner.runSchematic(
200+
'ssr',
201+
{ ...defaultOptions, serverRouting: undefined },
202+
appTree,
203+
);
204+
205+
expect(prompter).toHaveBeenCalledTimes(1);
206+
207+
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeFalse();
208+
});
153209
});
154210

155211
describe('Legacy browser builder', () => {
@@ -216,5 +272,26 @@ describe('SSR Schematic', () => {
216272
const content = tree.readContent('/projects/test-app/src/server.ts');
217273
expect(content).toContain(`const distFolder = join(process.cwd(), 'dist/test-app/browser');`);
218274
});
275+
276+
it('throws an exception when used with `serverRouting`', async () => {
277+
await expectAsync(
278+
schematicRunner.runSchematic('ssr', { ...defaultOptions, serverRouting: true }, appTree),
279+
).toBeRejectedWithError(/Server routing APIs.*`application` builder/);
280+
});
281+
282+
it('automatically disables `serverRouting` and does not prompt for it', async () => {
283+
const prompter = jasmine.createSpy<Prompt>('prompt').and.resolveTo(false);
284+
setPrompterForTestOnly(prompter);
285+
286+
const tree = await schematicRunner.runSchematic(
287+
'ssr',
288+
{ ...defaultOptions, serverRouting: undefined },
289+
appTree,
290+
);
291+
292+
expect(prompter).not.toHaveBeenCalled();
293+
294+
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeFalse();
295+
});
219296
});
220297
});

packages/schematics/angular/ssr/schema.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
},
1919
"serverRouting": {
2020
"description": "Creates a server application using the Server Routing and App Engine APIs (Developer Preview).",
21-
"x-prompt": "Would you like to use the Server Routing and App Engine APIs (Developer Preview) for this server application?",
22-
"type": "boolean",
23-
"default": false
21+
"type": "boolean"
2422
}
2523
},
2624
"required": ["project"],

0 commit comments

Comments
 (0)