mirror of
https://github.com/angular/angular-cli.git
synced 2025-05-16 18:43:42 +08:00
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.
This commit is contained in:
parent
9921271ce1
commit
160dee33d7
@ -14,6 +14,7 @@ import { join } from 'node:path';
|
||||
describe('@angular/ssr ng-add schematic', () => {
|
||||
const defaultOptions = {
|
||||
project: 'test-app',
|
||||
serverRouting: false,
|
||||
};
|
||||
|
||||
const schematicRunner = new SchematicTestRunner(
|
||||
|
@ -84,6 +84,7 @@ ts_library(
|
||||
"//packages/angular_devkit/schematics",
|
||||
"//packages/angular_devkit/schematics/tasks",
|
||||
"//packages/schematics/angular/third_party/github.com/Microsoft/TypeScript",
|
||||
"@npm//@inquirer/prompts",
|
||||
"@npm//@types/node",
|
||||
"@npm//browserslist",
|
||||
"@npm//jsonc-parser",
|
||||
|
@ -32,6 +32,7 @@ describe('Application Schematic', () => {
|
||||
const defaultOptions: ApplicationOptions = {
|
||||
name: 'foo',
|
||||
skipPackageJson: false,
|
||||
serverRouting: false,
|
||||
};
|
||||
|
||||
let workspaceTree: UnitTestTree;
|
||||
|
@ -38,6 +38,7 @@ import { ProjectDefinition, getWorkspace } from '../utility/workspace';
|
||||
import { Builders } from '../utility/workspace-models';
|
||||
|
||||
import { Schema as SSROptions } from './schema';
|
||||
import { isTTY } from './tty';
|
||||
|
||||
const SERVE_SSR_TARGET_NAME = 'serve-ssr';
|
||||
const PRERENDER_TARGET_NAME = 'prerender';
|
||||
@ -85,7 +86,7 @@ async function getApplicationBuilderOutputPaths(
|
||||
const { outputPath } = architectTarget.options;
|
||||
if (outputPath === null || outputPath === undefined) {
|
||||
throw new SchematicsException(
|
||||
`outputPath for ${projectName} ${target} target is undeined or null.`,
|
||||
`outputPath for ${projectName} ${target} target is undefined or null.`,
|
||||
);
|
||||
}
|
||||
|
||||
@ -361,19 +362,20 @@ function addServerFile(
|
||||
};
|
||||
}
|
||||
|
||||
export default function (options: SSROptions): Rule {
|
||||
export default function (inputOptions: SSROptions): Rule {
|
||||
return async (host, context) => {
|
||||
const browserEntryPoint = await getMainFilePath(host, options.project);
|
||||
const browserEntryPoint = await getMainFilePath(host, inputOptions.project);
|
||||
const isStandalone = isStandaloneApp(host, browserEntryPoint);
|
||||
|
||||
const workspace = await getWorkspace(host);
|
||||
const clientProject = workspace.projects.get(options.project);
|
||||
const clientProject = workspace.projects.get(inputOptions.project);
|
||||
if (!clientProject) {
|
||||
throw targetBuildNotFoundError();
|
||||
}
|
||||
|
||||
const isUsingApplicationBuilder = usingApplicationBuilder(clientProject);
|
||||
|
||||
const serverRouting = await isServerRoutingEnabled(isUsingApplicationBuilder, inputOptions);
|
||||
const options = { ...inputOptions, serverRouting };
|
||||
const sourceRoot = clientProject.sourceRoot ?? posix.join(clientProject.root, 'src');
|
||||
|
||||
return chain([
|
||||
@ -404,3 +406,54 @@ function usingApplicationBuilder(project: ProjectDefinition) {
|
||||
|
||||
return isUsingApplicationBuilder;
|
||||
}
|
||||
|
||||
// Wrap inquirer in a `prompt` function.
|
||||
export type Prompt = (message: string, defaultValue: boolean) => Promise<boolean>;
|
||||
const defaultPrompter: Prompt = async (message, defaultValue) => {
|
||||
const { confirm } = await import('@inquirer/prompts');
|
||||
|
||||
return await confirm({
|
||||
message,
|
||||
default: defaultValue,
|
||||
});
|
||||
};
|
||||
|
||||
// Allow the prompt functionality to be overridden to facilitate testing.
|
||||
let prompt = defaultPrompter;
|
||||
export function setPrompterForTestOnly(prompter?: Prompt): void {
|
||||
prompt = prompter ?? defaultPrompter;
|
||||
}
|
||||
|
||||
/** Returns whether or not server routing is enabled, potentially prompting the user if necessary. */
|
||||
async function isServerRoutingEnabled(
|
||||
isUsingApplicationBuilder: boolean,
|
||||
options: SSROptions,
|
||||
): Promise<boolean> {
|
||||
if (!isUsingApplicationBuilder) {
|
||||
if (options.serverRouting) {
|
||||
throw new SchematicsException(
|
||||
'Server routing APIs can only be added to a project using `application` builder.',
|
||||
);
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Use explicit option if provided.
|
||||
if (options.serverRouting !== undefined) {
|
||||
return options.serverRouting;
|
||||
}
|
||||
|
||||
const serverRoutingDefault = false;
|
||||
|
||||
// Use the default if not in an interactive terminal.
|
||||
if (!isTTY()) {
|
||||
return serverRoutingDefault;
|
||||
}
|
||||
|
||||
// Prompt the user if in an interactive terminal and no option was provided.
|
||||
return await prompt(
|
||||
'Would you like to use the Server Routing and App Engine APIs (Developer Preview) for this server application?',
|
||||
/* defaultValue */ serverRoutingDefault,
|
||||
);
|
||||
}
|
||||
|
@ -10,10 +10,12 @@ import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/te
|
||||
|
||||
import { join } from 'node:path';
|
||||
import { Schema as ServerOptions } from './schema';
|
||||
import { Prompt, setPrompterForTestOnly } from './index';
|
||||
|
||||
describe('SSR Schematic', () => {
|
||||
const defaultOptions: ServerOptions = {
|
||||
project: 'test-app',
|
||||
serverRouting: false,
|
||||
};
|
||||
|
||||
const schematicRunner = new SchematicTestRunner(
|
||||
@ -30,6 +32,10 @@ describe('SSR Schematic', () => {
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
setPrompterForTestOnly((message) => {
|
||||
return fail(`Unmocked prompt: ${message}`) as never;
|
||||
});
|
||||
|
||||
appTree = await schematicRunner.runExternalSchematic(
|
||||
'@schematics/angular',
|
||||
'workspace',
|
||||
@ -81,6 +87,8 @@ describe('SSR Schematic', () => {
|
||||
});
|
||||
|
||||
describe('standalone application', () => {
|
||||
const originalTty = process.env['NG_FORCE_TTY'];
|
||||
|
||||
beforeEach(async () => {
|
||||
appTree = await schematicRunner.runExternalSchematic(
|
||||
'@schematics/angular',
|
||||
@ -98,6 +106,10 @@ describe('SSR Schematic', () => {
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env['NG_FORCE_TTY'] = originalTty;
|
||||
});
|
||||
|
||||
it('should add script section in package.json', async () => {
|
||||
const tree = await schematicRunner.runSchematic('ssr', defaultOptions, appTree);
|
||||
const { scripts } = tree.readJson('/package.json') as { scripts: Record<string, string> };
|
||||
@ -150,6 +162,74 @@ describe('SSR Schematic', () => {
|
||||
server: 'node-server',
|
||||
});
|
||||
});
|
||||
|
||||
it('generates server routing configuration when enabled', async () => {
|
||||
const tree = await schematicRunner.runSchematic(
|
||||
'ssr',
|
||||
{ ...defaultOptions, serverRouting: true },
|
||||
appTree,
|
||||
);
|
||||
|
||||
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeTrue();
|
||||
});
|
||||
|
||||
it('does not generate server routing configuration when disabled', async () => {
|
||||
const tree = await schematicRunner.runSchematic(
|
||||
'ssr',
|
||||
{ ...defaultOptions, serverRouting: false },
|
||||
appTree,
|
||||
);
|
||||
|
||||
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeFalse();
|
||||
});
|
||||
|
||||
it('generates server routing configuration when prompt is accepted by the user', async () => {
|
||||
const prompter = jasmine.createSpy<Prompt>('prompt').and.resolveTo(true);
|
||||
setPrompterForTestOnly(prompter);
|
||||
|
||||
process.env['NG_FORCE_TTY'] = 'TRUE';
|
||||
const tree = await schematicRunner.runSchematic(
|
||||
'ssr',
|
||||
{ ...defaultOptions, serverRouting: undefined },
|
||||
appTree,
|
||||
);
|
||||
|
||||
expect(prompter).toHaveBeenCalledTimes(1);
|
||||
|
||||
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeTrue();
|
||||
});
|
||||
|
||||
it('does not generate server routing configuration when prompt is rejected by the user', async () => {
|
||||
const prompter = jasmine.createSpy<Prompt>('prompt').and.resolveTo(false);
|
||||
setPrompterForTestOnly(prompter);
|
||||
|
||||
process.env['NG_FORCE_TTY'] = 'TRUE';
|
||||
const tree = await schematicRunner.runSchematic(
|
||||
'ssr',
|
||||
{ ...defaultOptions, serverRouting: undefined },
|
||||
appTree,
|
||||
);
|
||||
|
||||
expect(prompter).toHaveBeenCalledTimes(1);
|
||||
|
||||
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeFalse();
|
||||
});
|
||||
|
||||
it('defaults to skipping server route generation when not in an interactive terminal', async () => {
|
||||
const prompter = jasmine.createSpy<Prompt>('prompt').and.resolveTo(false);
|
||||
setPrompterForTestOnly(prompter);
|
||||
|
||||
process.env['NG_FORCE_TTY'] = 'FALSE';
|
||||
const tree = await schematicRunner.runSchematic(
|
||||
'ssr',
|
||||
{ ...defaultOptions, serverRouting: undefined },
|
||||
appTree,
|
||||
);
|
||||
|
||||
expect(prompter).not.toHaveBeenCalled();
|
||||
|
||||
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeFalse();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Legacy browser builder', () => {
|
||||
@ -216,5 +296,27 @@ describe('SSR Schematic', () => {
|
||||
const content = tree.readContent('/projects/test-app/src/server.ts');
|
||||
expect(content).toContain(`const distFolder = join(process.cwd(), 'dist/test-app/browser');`);
|
||||
});
|
||||
|
||||
it('throws an exception when used with `serverRouting`', async () => {
|
||||
await expectAsync(
|
||||
schematicRunner.runSchematic('ssr', { ...defaultOptions, serverRouting: true }, appTree),
|
||||
).toBeRejectedWithError(/Server routing APIs.*`application` builder/);
|
||||
});
|
||||
|
||||
it('automatically disables `serverRouting` and does not prompt for it', async () => {
|
||||
const prompter = jasmine.createSpy<Prompt>('prompt').and.resolveTo(false);
|
||||
setPrompterForTestOnly(prompter);
|
||||
|
||||
process.env['NG_FORCE_TTY'] = 'TRUE';
|
||||
const tree = await schematicRunner.runSchematic(
|
||||
'ssr',
|
||||
{ ...defaultOptions, serverRouting: undefined },
|
||||
appTree,
|
||||
);
|
||||
|
||||
expect(prompter).not.toHaveBeenCalled();
|
||||
|
||||
expect(tree.exists('/projects/test-app/src/app/app.routes.server.ts')).toBeFalse();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
@ -18,9 +18,7 @@
|
||||
},
|
||||
"serverRouting": {
|
||||
"description": "Creates a server application using the Server Routing and App Engine APIs (Developer Preview).",
|
||||
"x-prompt": "Would you like to use the Server Routing and App Engine APIs (Developer Preview) for this server application?",
|
||||
"type": "boolean",
|
||||
"default": false
|
||||
"type": "boolean"
|
||||
}
|
||||
},
|
||||
"required": ["project"],
|
||||
|
22
packages/schematics/angular/ssr/tty.ts
Normal file
22
packages/schematics/angular/ssr/tty.ts
Normal file
@ -0,0 +1,22 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google LLC All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.dev/license
|
||||
*/
|
||||
|
||||
function _isTruthy(value: undefined | string): boolean {
|
||||
// Returns true if value is a string that is anything but 0 or false.
|
||||
return value !== undefined && value !== '0' && value.toUpperCase() !== 'FALSE';
|
||||
}
|
||||
|
||||
export function isTTY(): boolean {
|
||||
// If we force TTY, we always return true.
|
||||
const force = process.env['NG_FORCE_TTY'];
|
||||
if (force !== undefined) {
|
||||
return _isTruthy(force);
|
||||
}
|
||||
|
||||
return !!process.stdout.isTTY && !_isTruthy(process.env['CI']);
|
||||
}
|
@ -32,15 +32,28 @@ export default async function () {
|
||||
|
||||
// Forcibly remove in case another test doesn't clean itself up.
|
||||
await rimraf('node_modules/@angular/ssr');
|
||||
await ng(
|
||||
'add',
|
||||
'@angular/ssr',
|
||||
'--project',
|
||||
projectName,
|
||||
'--skip-confirmation',
|
||||
'--skip-install',
|
||||
'--server-routing',
|
||||
);
|
||||
if (useWebpackBuilder) {
|
||||
await ng(
|
||||
'add',
|
||||
'@angular/ssr',
|
||||
'--project',
|
||||
projectName,
|
||||
'--skip-confirmation',
|
||||
'--skip-install',
|
||||
// Server routing is not supported on `browser` builder.
|
||||
// '--server-routing',
|
||||
);
|
||||
} else {
|
||||
await ng(
|
||||
'add',
|
||||
'@angular/ssr',
|
||||
'--project',
|
||||
projectName,
|
||||
'--skip-confirmation',
|
||||
'--skip-install',
|
||||
'--server-routing',
|
||||
);
|
||||
}
|
||||
|
||||
await useSha();
|
||||
await installWorkspacePackages();
|
||||
|
@ -9,7 +9,13 @@ export default async function () {
|
||||
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
|
||||
// forcibly remove in case another test doesn't clean itself up
|
||||
await rimraf('node_modules/@angular/ssr');
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
|
||||
|
||||
if (useWebpackBuilder) {
|
||||
// `--server-routing` not supported in `browser` builder.
|
||||
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
|
||||
} else {
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
|
||||
}
|
||||
|
||||
await useSha();
|
||||
await installWorkspacePackages();
|
||||
|
@ -8,9 +8,16 @@ import { updateJsonFile, updateServerFileForWebpack, useSha } from '../../../uti
|
||||
export default async function () {
|
||||
// forcibly remove in case another test doesn't clean itself up
|
||||
await rimraf('node_modules/@angular/ssr');
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
|
||||
|
||||
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
|
||||
|
||||
if (useWebpackBuilder) {
|
||||
// `--server-routing` not supported in `browser` builder.
|
||||
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
|
||||
} else {
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
|
||||
}
|
||||
|
||||
if (!useWebpackBuilder) {
|
||||
// Disable prerendering
|
||||
await updateJsonFile('angular.json', (json) => {
|
||||
|
@ -4,11 +4,20 @@ import { killAllProcesses, ng } from '../../../utils/process';
|
||||
import { rimraf, writeMultipleFiles } from '../../../utils/fs';
|
||||
import { installWorkspacePackages } from '../../../utils/packages';
|
||||
import { ngServe, useSha } from '../../../utils/project';
|
||||
import { getGlobalVariable } from '../../../utils/env';
|
||||
|
||||
export default async function () {
|
||||
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
|
||||
|
||||
// Forcibly remove in case another test doesn't clean itself up.
|
||||
await rimraf('node_modules/@angular/ssr');
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation');
|
||||
if (useWebpackBuilder) {
|
||||
// `--server-routing` not supported in `browser` builder.
|
||||
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
|
||||
} else {
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
|
||||
}
|
||||
|
||||
await useSha();
|
||||
await installWorkspacePackages();
|
||||
|
||||
|
@ -3,11 +3,20 @@ import { ng } from '../../utils/process';
|
||||
import { appendToFile, rimraf } from '../../utils/fs';
|
||||
import { ngServe, useSha } from '../../utils/project';
|
||||
import { installWorkspacePackages } from '../../utils/packages';
|
||||
import { getGlobalVariable } from '../../utils/env';
|
||||
|
||||
export default async function () {
|
||||
const useWebpackBuilder = !getGlobalVariable('argv')['esbuild'];
|
||||
|
||||
// Forcibly remove in case another test doesn't clean itself up.
|
||||
await rimraf('node_modules/@angular/ssr');
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation');
|
||||
if (useWebpackBuilder) {
|
||||
// `--server-routing` not supported in `browser` builder.
|
||||
await ng('add', '@angular/ssr', '--skip-confirmation', '--skip-install');
|
||||
} else {
|
||||
await ng('add', '@angular/ssr', '--server-routing', '--skip-confirmation', '--skip-install');
|
||||
}
|
||||
|
||||
await useSha();
|
||||
await installWorkspacePackages();
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user