Skip to content

Commit b9890e9

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 b9890e9

File tree

10 files changed

+187
-21
lines changed

10 files changed

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

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"],

tests/legacy-cli/e2e/tests/build/prerender/discover-routes-ngmodule.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,28 @@ export default async function () {
3232

3333
// Forcibly remove in case another test doesn't clean itself up.
3434
await rimraf('node_modules/@angular/ssr');
35-
await ng(
36-
'add',
37-
'@angular/ssr',
38-
'--project',
39-
projectName,
40-
'--skip-confirmation',
41-
'--skip-install',
42-
'--server-routing',
43-
);
35+
if (useWebpackBuilder) {
36+
await ng(
37+
'add',
38+
'@angular/ssr',
39+
'--project',
40+
projectName,
41+
'--skip-confirmation',
42+
'--skip-install',
43+
// Server routing is not supported on `browser` builder.
44+
// '--server-routing',
45+
);
46+
} else {
47+
await ng(
48+
'add',
49+
'@angular/ssr',
50+
'--project',
51+
projectName,
52+
'--skip-confirmation',
53+
'--skip-install',
54+
'--server-routing',
55+
);
56+
}
4457

4558
await useSha();
4659
await installWorkspacePackages();

tests/legacy-cli/e2e/tests/build/server-rendering/express-engine-csp-nonce.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,13 @@ export default async function () {
99
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
1010
// forcibly remove in case another test doesn't clean itself up
1111
await rimraf('node_modules/@angular/ssr');
12-
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
12+
13+
if (useWebpackBuilder) {
14+
// `--server-routing` not supported in `browser` builder.
15+
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
16+
} else {
17+
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
18+
}
1319

1420
await useSha();
1521
await installWorkspacePackages();

tests/legacy-cli/e2e/tests/build/server-rendering/express-engine-standalone.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,16 @@ import { updateJsonFile, updateServerFileForWebpack, useSha } from '../../../uti
88
export default async function () {
99
// forcibly remove in case another test doesn't clean itself up
1010
await rimraf('node_modules/@angular/ssr');
11-
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
1211

1312
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
13+
14+
if (useWebpackBuilder) {
15+
// `--server-routing` not supported in `browser` builder.
16+
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
17+
} else {
18+
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
19+
}
20+
1421
if (!useWebpackBuilder) {
1522
// Disable prerendering
1623
await updateJsonFile('angular.json', (json) => {

tests/legacy-cli/e2e/tests/commands/serve/ssr-http-requests-assets.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,20 @@ import { killAllProcesses, ng } from '../../../utils/process';
44
import { rimraf, writeMultipleFiles } from '../../../utils/fs';
55
import { installWorkspacePackages } from '../../../utils/packages';
66
import { ngServe, useSha } from '../../../utils/project';
7+
import { getGlobalVariable } from '../../../utils/env';
78

89
export default async function () {
10+
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
11+
912
// Forcibly remove in case another test doesn't clean itself up.
1013
await rimraf('node_modules/@angular/ssr');
11-
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation');
14+
if (useWebpackBuilder) {
15+
// `--server-routing` not supported in `browser` builder.
16+
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
17+
} else {
18+
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
19+
}
20+
1221
await useSha();
1322
await installWorkspacePackages();
1423

tests/legacy-cli/e2e/tests/vite/ssr-error-stack.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,20 @@ import { ng } from '../../utils/process';
33
import { appendToFile, rimraf } from '../../utils/fs';
44
import { ngServe, useSha } from '../../utils/project';
55
import { installWorkspacePackages } from '../../utils/packages';
6+
import { getGlobalVariable } from '../../utils/env';
67

78
export default async function () {
9+
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
10+
811
// Forcibly remove in case another test doesn't clean itself up.
912
await rimraf('node_modules/@angular/ssr');
10-
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation');
13+
if (useWebpackBuilder) {
14+
// `--server-routing` not supported in `browser` builder.
15+
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
16+
} else {
17+
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
18+
}
19+
1120
await useSha();
1221
await installWorkspacePackages();
1322

0 commit comments

Comments
 (0)