Skip to content

Commit 3d833d3

Browse files
authored
fix(toolkit-lib): unnecessary dependency on promptly (#523)
Fixes #396 This PR refactors the resource import prompts to use the IoHost instead. With that change, we can remove the promptly library as a toolkit-lib dependency and only have it in the CLI. The change itself is not in a code path that is currently publicly available to toolkit-lib users, it only affects the `cdk import` command. The user experience for this is virtually unchanged, see screenshots. Before: <img width="691" alt="image" src="https://github.com/user-attachments/assets/1f13644a-e8bc-4a7b-bfca-bf23fdfb9ceb" /> <img width="956" alt="image" src="https://github.com/user-attachments/assets/57181fa6-4c20-4b81-9ce7-c1451f62e033" /> <img width="1224" alt="image" src="https://github.com/user-attachments/assets/ee2768da-1eca-42b8-8c2e-c00772f07d99" /> After: <img width="681" alt="image" src="https://github.com/user-attachments/assets/f5185293-d381-44ad-94a5-76b47224b703" /> <img width="954" alt="image" src="https://github.com/user-attachments/assets/1a4e5a6b-ec89-469e-9b8e-8e2e4cbaf950" /> <img width="1110" alt="image" src="https://github.com/user-attachments/assets/e0d43e3b-91d6-4f7b-9c65-fae3387c874c" /> --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent b1dfe96 commit 3d833d3

File tree

14 files changed

+151
-58
lines changed

14 files changed

+151
-58
lines changed

.projenrc.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,6 @@ const toolkitLib = configureProject(
751751
'glob',
752752
'minimatch',
753753
'p-limit@^3',
754-
'promptly',
755754
'proxy-agent',
756755
'semver',
757756
'split2',
@@ -773,6 +772,8 @@ const toolkitLib = configureProject(
773772
'aws-sdk-client-mock-jest',
774773
'fast-check',
775774
'jest-environment-node',
775+
'@types/jest-when',
776+
'jest-when',
776777
'nock@13',
777778
'typedoc',
778779
'xml-js',

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/import/cdk-import-interactive.integtest.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ integTest('cdk import prompts the user for sns topic arns', withDefaultFixture(a
2525
await fixture.cdk(['import', fullStackName], {
2626
interact: [
2727
{
28-
prompt: /Topic1.*\(empty to skip\):/,
28+
prompt: /Topic1.*\(empty to skip\)/,
2929
input: topic1Arn,
3030
},
3131
{
32-
prompt: /Topic2.*\(empty to skip\):/,
32+
prompt: /Topic2.*\(empty to skip\)/,
3333
input: topic2Arn,
3434
},
3535
],

packages/@aws-cdk/toolkit-lib/.projen/deps.json

Lines changed: 8 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/toolkit-lib/.projen/tasks.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/toolkit-lib/docs/message-registry.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ Please let us know by [opening an issue](https://github.com/aws/aws-cdk-cli/issu
6767
| `CDK_TOOLKIT_I1901` | Provides stack data | `result` | {@link StackAndAssemblyData} |
6868
| `CDK_TOOLKIT_I1902` | Successfully deployed stacks | `result` | {@link AssemblyData} |
6969
| `CDK_TOOLKIT_I2901` | Provides details on the selected stacks and their dependencies | `result` | {@link StackDetailsPayload} |
70+
| `CDK_TOOLKIT_I3100` | Confirm the import of a specific resource | `info` | {@link ResourceImportRequest} |
71+
| `CDK_TOOLKIT_I3110` | Additional information is needed to identify a resource | `info` | {@link ResourceIdentificationRequest} |
7072
| `CDK_TOOLKIT_E3900` | Resource import failed | `error` | {@link ErrorPayload} |
7173
| `CDK_TOOLKIT_I4000` | Diff stacks is starting | `trace` | {@link StackSelectionDetails} |
7274
| `CDK_TOOLKIT_I4001` | Output of the diff command | `info` | {@link DiffResult} |

packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { BuildAsset, DeployConfirmationRequest, PublishAsset, StackDeployPr
88
import type { StackDestroy, StackDestroyProgress } from '../../../payloads/destroy';
99
import type { AssetBatchDeletionRequest } from '../../../payloads/gc';
1010
import type { HotswapDeploymentDetails, HotswapDeploymentAttempt, HotswappableChange, HotswapResult } from '../../../payloads/hotswap';
11+
import type { ResourceIdentificationRequest, ResourceImportRequest } from '../../../payloads/import';
1112
import type { StackDetailsPayload } from '../../../payloads/list';
1213
import type { CloudWatchLogEvent, CloudWatchLogMonitorControlEvent } from '../../../payloads/logs-monitor';
1314
import type { RefactorResult } from '../../../payloads/refactor';
@@ -61,6 +62,16 @@ export const IO = {
6162
}),
6263

6364
// 3: Import & Migrate
65+
CDK_TOOLKIT_I3100: make.confirm<ResourceImportRequest>({
66+
code: 'CDK_TOOLKIT_I3100',
67+
description: 'Confirm the import of a specific resource',
68+
interface: 'ResourceImportRequest',
69+
}),
70+
CDK_TOOLKIT_I3110: make.question<ResourceIdentificationRequest>({
71+
code: 'CDK_TOOLKIT_I3110',
72+
description: 'Additional information is needed to identify a resource',
73+
interface: 'ResourceIdentificationRequest',
74+
}),
6475
CDK_TOOLKIT_E3900: make.error<ErrorPayload>({
6576
code: 'CDK_TOOLKIT_E3900',
6677
description: 'Resource import failed',

packages/@aws-cdk/toolkit-lib/lib/api/resource-import/importer.ts

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import type * as cxapi from '@aws-cdk/cx-api';
55
import type { ResourceIdentifierSummary, ResourceToImport } from '@aws-sdk/client-cloudformation';
66
import * as chalk from 'chalk';
77
import * as fs from 'fs-extra';
8-
import * as promptly from 'promptly';
98
import type { DeploymentMethod } from '../../actions/deploy';
109
import { ToolkitError } from '../../toolkit/toolkit-error';
1110
import type { Deployments } from '../deployments';
@@ -347,10 +346,14 @@ export class ResourceImporter {
347346
const candidateProps = Object.fromEntries(satisfiedPropSet.map(p => [p, resourceProps[p]]));
348347
const displayCandidateProps = fmtdict(candidateProps);
349348

350-
if (await promptly.confirm(
351-
`${chalk.blue(resourceName)} (${resourceType}): import with ${chalk.yellow(displayCandidateProps)} (yes/no) [default: yes]? `,
352-
{ default: 'yes' },
353-
)) {
349+
const importTheResource = await this.ioHelper.requestResponse(IO.CDK_TOOLKIT_I3100.req(`${chalk.blue(resourceName)} (${resourceType}): import with ${chalk.yellow(displayCandidateProps)}`, {
350+
resource: {
351+
type: resourceType,
352+
props: candidateProps,
353+
stringifiedProps: displayCandidateProps,
354+
},
355+
}));
356+
if (importTheResource) {
354357
return candidateProps;
355358
}
356359
}
@@ -364,35 +367,32 @@ export class ResourceImporter {
364367
// We cannot auto-import this, ask the user for one of the props
365368
// The only difference between these cases is what we print: for multiple properties, we print a preamble
366369
const prefix = `${chalk.blue(resourceName)} (${resourceType})`;
367-
let preamble;
368-
let promptPattern;
370+
const promptPattern = `${prefix}: enter %s`;
369371
if (idPropSets.length > 1) {
370-
preamble = `${prefix}: enter one of ${idPropSets.map(x => chalk.blue(x.join('+'))).join(', ')} to import (all empty to skip)`;
371-
promptPattern = `${prefix}: enter %`;
372-
} else {
373-
promptPattern = `${prefix}: enter %`;
372+
const preamble = `${prefix}: enter one of ${idPropSets.map(x => chalk.blue(x.join('+'))).join(', ')} to import (leave all empty to skip)`;
373+
await this.ioHelper.defaults.info(preamble);
374374
}
375375

376376
// Do the input loop here
377-
if (preamble) {
378-
await this.ioHelper.defaults.info(preamble);
379-
}
380377
for (const idProps of idPropSets) {
381378
const input: Record<string, string> = {};
382379
for (const idProp of idProps) {
383380
// If we have a value from the template, use it as default. This will only be a partial
384381
// identifier if present, otherwise we would have done the import already above.
385382
const defaultValue = resourceProps[idProp] ?? '';
386383

387-
const prompt = [
388-
promptPattern.replace(/%/g, chalk.blue(idProp)),
389-
defaultValue
390-
? `[${defaultValue}]`
391-
: '(empty to skip)',
392-
].join(' ') + ':';
393-
const response = await promptly.prompt(prompt,
394-
{ default: defaultValue, trim: true },
395-
);
384+
const response = await this.ioHelper.requestResponse(IO.CDK_TOOLKIT_I3110.req(
385+
format(promptPattern, chalk.blue(idProp)),
386+
{
387+
resource: {
388+
name: resourceName,
389+
type: resourceType,
390+
idProp,
391+
},
392+
responseDescription: defaultValue ? undefined : 'empty to skip',
393+
},
394+
defaultValue,
395+
));
396396

397397
if (!response) {
398398
break;
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import type { DataRequest } from './types';
2+
3+
/**
4+
* A proposed resource import.
5+
*/
6+
export interface ResourceImportRequest {
7+
/**
8+
* The resource to be imported
9+
*/
10+
readonly resource: {
11+
/**
12+
* The CloudFormation resource type of the resource
13+
*/
14+
readonly type: string;
15+
/**
16+
* The properties of the imported resource
17+
*/
18+
readonly props: Record<string, any>;
19+
/**
20+
* A formattated string representation of the props.
21+
*/
22+
readonly stringifiedProps: string;
23+
};
24+
}
25+
26+
/**
27+
* A resource that needs to be identified during an import.
28+
*/
29+
export interface ResourceIdentificationRequest extends DataRequest {
30+
/**
31+
* The resource that needs to be identified.
32+
*/
33+
readonly resource: {
34+
/**
35+
* The construct path or logical id of the resource.
36+
*/
37+
readonly name: string;
38+
/**
39+
* The type of the resource.
40+
*/
41+
readonly type: string;
42+
/**
43+
* The property that we try to identify the resource by.
44+
*/
45+
readonly idProp: string;
46+
};
47+
}

packages/@aws-cdk/toolkit-lib/lib/payloads/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,4 @@ export * from './diff';
1616
export * from './logs-monitor';
1717
export * from './hotswap';
1818
export * from './gc';
19+
export * from './import';

packages/@aws-cdk/toolkit-lib/package.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/toolkit-lib/test/_helpers/test-io-host.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { IIoHost, IoMessage, IoMessageLevel, IoRequest } from '../../lib/api/io';
1+
import { when } from 'jest-when';
2+
import type { IIoHost, IoMessage, IoMessageCode, IoMessageLevel, IoRequest } from '../../lib/api/io';
23
import type { IoHelper } from '../../lib/api/io/private';
34
import { asIoHelper, isMessageRelevantForLevel } from '../../lib/api/io/private';
45

@@ -66,4 +67,26 @@ export class TestIoHost implements IIoHost {
6667
message: expect.stringContaining(m.containing),
6768
}));
6869
}
70+
71+
/**
72+
* Mocks the response for a given message code.
73+
*
74+
* Use `requestSpy.mockReset()` to remove mock.
75+
*/
76+
public mockResponse(code: IoMessageCode, response: any) {
77+
when(this.requestSpy)
78+
.calledWith(expect.objectContaining({ code }))
79+
.mockResolvedValue(response);
80+
}
81+
82+
/**
83+
* Mocks the response for a given message code, only once.
84+
*
85+
* Use `requestSpy.mockReset()` to remove mock.
86+
*/
87+
public mockResponseOnce(code: IoMessageCode, response: any) {
88+
when(this.requestSpy)
89+
.calledWith(expect.objectContaining({ code }))
90+
.mockResolvedValueOnce(response);
91+
}
6992
}

0 commit comments

Comments
 (0)