Skip to content

Commit 90e1f1f

Browse files
committed
address PR comments
1 parent f8d75c0 commit 90e1f1f

File tree

6 files changed

+130
-94
lines changed

6 files changed

+130
-94
lines changed

packages/aws-cdk/lib/cli/cli-config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export async function makeConfig(): Promise<CliConfig> {
128128
default: { type: 'boolean', desc: 'Change flags to default state', requiresArg: false },
129129
interactive: { type: 'boolean', alias: ['i'], desc: 'Interactive option for the flags command', requiresArg: false },
130130
safe: { type: 'boolean', desc: 'Enable all feature flags that do not impact the user\'s application', requiresArg: false },
131-
concurrency: { type: 'number', desc: 'Maximum number of simultaneous synths to execute.', default: 4, requiresArg: true },
131+
concurrency: { type: 'number', alias: ['conc'], desc: 'Maximum number of simultaneous synths to execute.', default: 4, requiresArg: true },
132132
},
133133
},
134134
'deploy': {

packages/aws-cdk/lib/cli/cli-type-registry.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,9 @@
381381
},
382382
"concurrency": {
383383
"type": "number",
384+
"alias": [
385+
"conc"
386+
],
384387
"desc": "Maximum number of simultaneous synths to execute.",
385388
"default": 4,
386389
"requiresArg": true

packages/aws-cdk/lib/cli/parse-command-line-arguments.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,7 @@ export function parseCommandLineArguments(args: Array<string>): any {
406406
.option('concurrency', {
407407
default: 4,
408408
type: 'number',
409+
alias: ['conc'],
409410
desc: 'Maximum number of simultaneous synths to execute.',
410411
requiresArg: true,
411412
}),

packages/aws-cdk/lib/cli/user-input.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,8 @@ export interface FlagsOptions {
676676
/**
677677
* Maximum number of simultaneous synths to execute.
678678
*
679+
* aliases: conc
680+
*
679681
* @default - 4
680682
*/
681683
readonly concurrency?: number;

packages/aws-cdk/lib/commands/flag-operations.ts

Lines changed: 97 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,12 @@ interface FlagOperationsParams {
4343
unconfigured?: boolean;
4444
safe?: boolean;
4545
concurrency?: number;
46+
47+
/** User provided --app option */
48+
app?: string;
4649
}
4750

48-
export async function handleFlags(flagData: FeatureFlag[], ioHelper: IoHelper, options: FlagsOptions, toolkit: Toolkit) {
51+
export async function handleFlags(flagData: FeatureFlag[], ioHelper: IoHelper, options: FlagsOptions & { app?: string }, toolkit: Toolkit) {
4952
flagData = flagData.filter(flag => !OBSOLETE_FLAGS.includes(flag.name));
5053

5154
if (flagData.length == 0) {
@@ -65,6 +68,7 @@ export async function handleFlags(flagData: FeatureFlag[], ioHelper: IoHelper, o
6568
unconfigured: options.unconfigured,
6669
safe: options.safe,
6770
concurrency: options.concurrency,
71+
app: options.app,
6872
};
6973

7074
const interactiveOptions = Object.values(FlagsMenuOptions);
@@ -249,41 +253,82 @@ async function setFlag(params: FlagOperationsParams, interactive?: boolean) {
249253
}
250254
}
251255

252-
async function testFlagSafety(
253-
flag: FeatureFlag,
256+
async function setSafeFlags(params: FlagOperationsParams): Promise<void> {
257+
const { flagData, toolkit, ioHelper, concurrency, app: appOption } = params;
258+
259+
const cdkJson = await JSON.parse(await fs.readFile(path.join(process.cwd(), 'cdk.json'), 'utf-8'));
260+
const app = appOption || cdkJson.app;
261+
262+
const isUsingTsNode = app.includes('ts-node');
263+
264+
if (isUsingTsNode && !app.includes('-T') && !app.includes('--transpileOnly')) {
265+
await ioHelper.defaults.info('Repeated synths with ts-node will type-check the application on every synth. Add --transpileOnly to cdk.json\'s "app" command to make this operation faster.');
266+
}
267+
268+
const unconfiguredFlags = flagData.filter(flag =>
269+
flag.userValue === undefined &&
270+
isBooleanFlag(flag) &&
271+
(flag.unconfiguredBehavesLike?.v2 !== flag.recommendedValue),
272+
);
273+
274+
if (unconfiguredFlags.length === 0) {
275+
await ioHelper.defaults.info('No unconfigured feature flags found.');
276+
return;
277+
}
278+
279+
const baseContext = new CdkAppMultiContext(process.cwd());
280+
const baseContextValues = await baseContext.read();
281+
282+
const baseSource = await toolkit.fromCdkApp(app, {
283+
contextStore: baseContext,
284+
outdir: path.join(process.cwd(), 'baseline'),
285+
});
286+
287+
const baseCx = await toolkit.synth(baseSource);
288+
const baseAssembly = baseCx.cloudAssembly;
289+
const allStacks = baseAssembly.stacksRecursively;
290+
291+
const queue = new PQueue({ concurrency: concurrency });
292+
293+
const safeFlags = await batchTestFlags(unconfiguredFlags, baseContextValues, toolkit, app, allStacks, queue);
294+
295+
await fs.remove(path.join(process.cwd(), 'baseline'));
296+
297+
if (safeFlags.length > 0) {
298+
await ioHelper.defaults.info('Safe flags that can be set without template changes:');
299+
for (const flag of safeFlags) {
300+
await ioHelper.defaults.info(`- ${flag.name} -> ${flag.recommendedValue}`);
301+
}
302+
303+
await handleUserResponse(params, safeFlags.map(flag => flag.name));
304+
} else {
305+
await ioHelper.defaults.info('No flags can be safely set without causing template changes.');
306+
}
307+
}
308+
309+
async function batchTestFlags(
310+
flags: FeatureFlag[],
254311
baseContextValues: Record<string, any>,
255312
toolkit: Toolkit,
256313
app: string,
257314
allStacks: any[],
258-
): Promise<boolean> {
259-
const testContext = new MemoryContext(baseContextValues);
260-
const newValue = toBooleanValue(flag.recommendedValue);
261-
await testContext.update({ [flag.name]: newValue });
315+
queue: PQueue,
316+
): Promise<FeatureFlag[]> {
317+
if (flags.length === 0) {
318+
return [];
319+
}
262320

263-
const testSource = await toolkit.fromCdkApp(app, {
264-
contextStore: testContext,
265-
outdir: path.join(process.cwd(), `test-${flag.name}`),
321+
const allFlagsContext = { ...baseContextValues };
322+
flags.forEach(flag => {
323+
allFlagsContext[flag.name] = toBooleanValue(flag.recommendedValue);
266324
});
267325

268-
const testCx = await toolkit.synth(testSource);
269-
270-
for (const stack of allStacks) {
271-
const templatePath = stack.templateFullPath;
272-
const diff = await toolkit.diff(testCx, {
273-
method: DiffMethod.LocalFile(templatePath),
274-
stacks: {
275-
strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE,
276-
patterns: [stack.hierarchicalId],
277-
},
278-
});
279-
280-
for (const stackDiff of Object.values(diff)) {
281-
if (stackDiff.differenceCount > 0) {
282-
return false;
283-
}
284-
}
326+
const allSafe = await testBatch(allFlagsContext, toolkit, app, allStacks, 'batch-all');
327+
if (allSafe) {
328+
return flags;
285329
}
286-
return true;
330+
331+
return isolateUnsafeFlags(flags, baseContextValues, toolkit, app, allStacks, queue);
287332
}
288333

289334
async function testBatch(
@@ -373,81 +418,41 @@ async function isolateUnsafeFlags(
373418
return safeFlags;
374419
}
375420

376-
async function batchTestFlags(
377-
flags: FeatureFlag[],
421+
async function testFlagSafety(
422+
flag: FeatureFlag,
378423
baseContextValues: Record<string, any>,
379424
toolkit: Toolkit,
380425
app: string,
381426
allStacks: any[],
382-
queue: PQueue,
383-
): Promise<FeatureFlag[]> {
384-
if (flags.length === 0) {
385-
return [];
386-
}
387-
388-
const allFlagsContext = { ...baseContextValues };
389-
flags.forEach(flag => {
390-
allFlagsContext[flag.name] = toBooleanValue(flag.recommendedValue);
391-
});
392-
393-
const allSafe = await testBatch(allFlagsContext, toolkit, app, allStacks, 'batch-all');
394-
if (allSafe) {
395-
return flags;
396-
}
397-
398-
return isolateUnsafeFlags(flags, baseContextValues, toolkit, app, allStacks, queue);
399-
}
400-
401-
async function setSafeFlags(params: FlagOperationsParams): Promise<void> {
402-
const { flagData, toolkit, ioHelper, concurrency } = params;
403-
404-
const cdkJson = await JSON.parse(await fs.readFile(path.join(process.cwd(), 'cdk.json'), 'utf-8'));
405-
const app = cdkJson.app;
406-
407-
const isUsingTsNode = app.includes('ts-node');
408-
if (isUsingTsNode && !app.includes('-T') && !app.includes('--transpileOnly')) {
409-
await ioHelper.defaults.info('You are currently running with ts-node. Adding --transpileOnly may make this operation faster.');
410-
}
411-
412-
const unconfiguredFlags = flagData.filter(flag =>
413-
flag.userValue === undefined &&
414-
isBooleanFlag(flag) &&
415-
(flag.unconfiguredBehavesLike?.v2 !== flag.recommendedValue),
416-
);
417-
418-
if (unconfiguredFlags.length === 0) {
419-
await ioHelper.defaults.info('No unconfigured feature flags found.');
420-
return;
421-
}
422-
423-
const baseContext = new CdkAppMultiContext(process.cwd());
424-
const baseContextValues = await baseContext.read();
427+
): Promise<boolean> {
428+
const testContext = new MemoryContext(baseContextValues);
429+
const newValue = toBooleanValue(flag.recommendedValue);
430+
await testContext.update({ [flag.name]: newValue });
425431

426-
const baseSource = await toolkit.fromCdkApp(app, {
427-
contextStore: baseContext,
428-
outdir: path.join(process.cwd(), 'baseline'),
432+
const testSource = await toolkit.fromCdkApp(app, {
433+
contextStore: testContext,
434+
outdir: path.join(process.cwd(), `test-${flag.name}`),
429435
});
430436

431-
const baseCx = await toolkit.synth(baseSource);
432-
const baseAssembly = baseCx.cloudAssembly;
433-
const allStacks = baseAssembly.stacksRecursively;
434-
435-
const queue = new PQueue({ concurrency: concurrency });
436-
437-
const safeFlags = await batchTestFlags(unconfiguredFlags, baseContextValues, toolkit, app, allStacks, queue);
437+
const testCx = await toolkit.synth(testSource);
438438

439-
await fs.remove(path.join(process.cwd(), 'baseline'));
439+
for (const stack of allStacks) {
440+
const templatePath = stack.templateFullPath;
441+
const diff = await toolkit.diff(testCx, {
442+
method: DiffMethod.LocalFile(templatePath),
443+
stacks: {
444+
strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE,
445+
patterns: [stack.hierarchicalId],
446+
},
447+
});
440448

441-
if (safeFlags.length > 0) {
442-
await ioHelper.defaults.info('Safe flags that can be set without template changes:');
443-
for (const flag of safeFlags) {
444-
await ioHelper.defaults.info(`- ${flag.name} -> ${flag.recommendedValue}`);
449+
for (const stackDiff of Object.values(diff)) {
450+
if (stackDiff.differenceCount > 0) {
451+
return false;
452+
}
445453
}
446-
447-
await handleUserResponse(params, safeFlags.map(flag => flag.name));
448-
} else {
449-
await ioHelper.defaults.info('No flags can be safely set without causing template changes.');
450454
}
455+
return true;
451456
}
452457

453458
async function prototypeChanges(

packages/aws-cdk/test/commands/flag-operations.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,32 @@ describe('setSafeFlags', () => {
776776
await handleFlags(mockFlagsData, ioHelper, options, mockToolkit);
777777

778778
const plainTextOutput = output();
779-
expect(plainTextOutput).toContain('You are currently running with ts-node. Adding --transpileOnly may make this operation faster.');
779+
expect(plainTextOutput).toContain('Repeated synths with ts-node will type-check the application on every synth. Add --transpileOnly to cdk.json\'s "app" command to make this operation faster.');
780+
781+
await cleanupCdkJsonFile(cdkJsonPath);
782+
requestResponseSpy.mockRestore();
783+
});
784+
785+
test('shows ts-node performance tip when user supplies --app option with ts-node', async () => {
786+
const cdkJsonPath = await createCdkJsonFile({});
787+
788+
mockToolkit.diff.mockResolvedValue({
789+
TestStack: { differenceCount: 0 } as any,
790+
});
791+
792+
const requestResponseSpy = jest.spyOn(ioHelper, 'requestResponse');
793+
requestResponseSpy.mockResolvedValue(false);
794+
795+
const options: FlagsOptions & { app?: string } = {
796+
safe: true,
797+
concurrency: 4,
798+
app: 'npx ts-node bin/app.ts',
799+
};
800+
801+
await handleFlags(mockFlagsData, ioHelper, options, mockToolkit);
802+
803+
const plainTextOutput = output();
804+
expect(plainTextOutput).toContain('Repeated synths with ts-node will type-check the application on every synth. Add --transpileOnly to cdk.json\'s "app" command to make this operation faster.');
780805

781806
await cleanupCdkJsonFile(cdkJsonPath);
782807
requestResponseSpy.mockRestore();

0 commit comments

Comments
 (0)