Skip to content

Commit 1f0ef3b

Browse files
imhoffdimhoffd
authored
fix(serve): fix unclosed connection issue again (#3500)
Keeping BottomBar open seems to be a consistent issue on Windows. BottomBar creates a readline interface, so SIGINT has to go through it, and the signal doesn't seem to bubble up properly, which means the Ionic CLI cannot properly clean up resources during a Ctrl-C, leading to serve issues where the underlying CLI is not terminated.
1 parent e56b054 commit 1f0ef3b

File tree

29 files changed

+299
-339
lines changed

29 files changed

+299
-339
lines changed

‎packages/@ionic/cli-framework/src/definitions.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as os from 'os';
22

33
import { Opts as ParseArgsOptions, ParsedArgs } from 'minimist';
44

5-
import { AliasedMap } from './utils/object';
5+
import * as ζobject from './utils/object';
66

77
export type NetworkInterface = { device: string; } & os.NetworkInterfaceInfo;
88

@@ -70,8 +70,8 @@ export interface ICommand<C extends ICommand<C, N, M, I, O>, N extends INamespac
7070

7171
export type CommandMapGetter<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption> = () => Promise<C>;
7272
export type NamespaceMapGetter<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption> = () => Promise<N>;
73-
export type ICommandMap<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption> = AliasedMap<string, CommandMapGetter<C, N, M, I, O>>;
74-
export type INamespaceMap<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption> = AliasedMap<string, NamespaceMapGetter<C, N, M, I, O>>;
73+
export type ICommandMap<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption> = ζobject.AliasedMap<string, CommandMapGetter<C, N, M, I, O>>;
74+
export type INamespaceMap<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption> = ζobject.AliasedMap<string, NamespaceMapGetter<C, N, M, I, O>>;
7575

7676
export interface INamespace<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption> {
7777
root: N;
@@ -139,11 +139,3 @@ export interface ValidationError {
139139
message: string;
140140
validator: Validator;
141141
}
142-
143-
export interface OutputStrategy {
144-
readonly stream: NodeJS.WritableStream;
145-
}
146-
147-
export interface RedrawLine {
148-
redrawLine(msg?: string): void;
149-
}

‎packages/@ionic/cli-framework/src/guards.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { CommandMetadata, CommandMetadataInput, CommandMetadataOption, HydratedCommandMetadata, ICommand, INamespace, PackageJson, RedrawLine } from './definitions';
1+
import { CommandMetadata, CommandMetadataInput, CommandMetadataOption, HydratedCommandMetadata, ICommand, INamespace, PackageJson } from './definitions';
22

33
export function isNamespace<C extends ICommand<C, N, M, I, O>, N extends INamespace<C, N, M, I, O>, M extends CommandMetadata<I, O>, I extends CommandMetadataInput, O extends CommandMetadataOption>(obj: any): obj is N {
44
return obj &&
@@ -26,7 +26,3 @@ export function isHydratedCommandMetadata<C extends ICommand<C, N, M, I, O>, N e
2626
export function isPackageJson(obj: any): obj is PackageJson {
2727
return obj && typeof obj.name === 'string';
2828
}
29-
30-
export function isRedrawLine(obj: any): obj is RedrawLine {
31-
return obj && typeof obj.redrawLine === 'function';
32-
}

‎packages/@ionic/cli-framework/src/lib/__tests__/prompts.ts

Lines changed: 3 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -185,61 +185,12 @@ describe('@ionic/cli-framework', () => {
185185
expect(onFallbackSpy).toHaveBeenCalledWith(question);
186186
});
187187

188-
describe('open', () => {
188+
describe('_inquirer', () => {
189189

190-
it('should call mute', async () => {
190+
it('should get _inquirer module from prompt module', async () => {
191191
const prompts = setupPromptMocks({ tty: true });
192192
const prompt = await prompts.createPromptModule();
193-
prompt.open();
194-
expect(mockMute).toHaveBeenCalledTimes(1);
195-
});
196-
197-
it('should call mute once if opened multiple times', async () => {
198-
const prompts = setupPromptMocks({ tty: true });
199-
const prompt = await prompts.createPromptModule();
200-
prompt.open();
201-
prompt.open();
202-
prompt.open();
203-
expect(mockMute).toHaveBeenCalledTimes(1);
204-
});
205-
206-
});
207-
208-
describe('close', () => {
209-
210-
it('should not close bottomBar if not opened', async () => {
211-
const prompts = setupPromptMocks({ tty: true });
212-
const prompt = await prompts.createPromptModule();
213-
prompt.close();
214-
expect(mockClose).not.toHaveBeenCalled();
215-
});
216-
217-
it('should close bottomBar if opened', async () => {
218-
const prompts = setupPromptMocks({ tty: true });
219-
const prompt = await prompts.createPromptModule();
220-
prompt.open();
221-
prompt.close();
222-
expect(mockClose).toHaveBeenCalledTimes(1);
223-
});
224-
225-
it('should close bottomBar once if closed multiple times', async () => {
226-
const prompts = setupPromptMocks({ tty: true });
227-
const prompt = await prompts.createPromptModule();
228-
prompt.open();
229-
prompt.close();
230-
prompt.close();
231-
expect(mockClose).toHaveBeenCalledTimes(1);
232-
});
233-
234-
});
235-
236-
describe('output', () => {
237-
238-
it('should get output stream from prompt module', async () => {
239-
const prompts = setupPromptMocks({ tty: true });
240-
const prompt = await prompts.createPromptModule();
241-
expect(prompt.output).toBeDefined();
242-
expect(prompt.output.stream).toBe(mockLogStream);
193+
expect(prompt._inquirer).toBeDefined();
243194
});
244195

245196
});

‎packages/@ionic/cli-framework/src/lib/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export * from './executor';
66
export * from './help';
77
export * from './logger';
88
export * from './options';
9+
export * from './output';
910
export * from './prompts';
1011
export * from './tasks';
1112
export * from './validators';

‎packages/@ionic/cli-framework/src/lib/output.ts

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import * as Debug from 'debug';
2+
import * as ζinquirer from 'inquirer';
3+
4+
import { Colors, DEFAULT_COLORS } from './colors';
5+
import { ICON_FAILURE, ICON_SUCCESS, Spinner, TaskChain } from './tasks';
6+
7+
const debug = Debug('ionic:cli-framework:lib:output');
8+
9+
export interface OutputStrategy {
10+
readonly stream: NodeJS.WritableStream;
11+
createTaskChain(): TaskChain;
12+
}
13+
14+
export interface RedrawLine {
15+
redrawLine(msg?: string): void;
16+
open(): void;
17+
close(): void;
18+
}
19+
20+
export interface StreamOutputStrategyOptions {
21+
readonly stream: NodeJS.WritableStream;
22+
readonly colors?: Colors;
23+
}
24+
25+
export class StreamOutputStrategy implements OutputStrategy {
26+
readonly stream: NodeJS.WritableStream;
27+
28+
protected readonly colors: Colors;
29+
30+
constructor({ stream = process.stdout, colors = DEFAULT_COLORS }: StreamOutputStrategyOptions) {
31+
this.stream = stream;
32+
this.colors = colors;
33+
}
34+
35+
createTaskChain(): TaskChain {
36+
const { failure, success } = this.colors;
37+
const chain = new TaskChain();
38+
39+
chain.on('next', task => {
40+
task.on('success', () => {
41+
this.stream.write(`${success(ICON_SUCCESS)} ${task.msg} - done!`);
42+
});
43+
44+
task.on('failure', () => {
45+
this.stream.write(`${failure(ICON_FAILURE)} ${task.msg} - failed!`);
46+
});
47+
});
48+
49+
return chain;
50+
}
51+
}
52+
53+
export interface BottomBarOutputStrategyOptions {
54+
readonly BottomBar: typeof ζinquirer.ui.BottomBar;
55+
readonly input?: NodeJS.ReadableStream;
56+
readonly output?: NodeJS.WritableStream;
57+
readonly colors?: Colors;
58+
}
59+
60+
export class BottomBarOutputStrategy implements OutputStrategy, RedrawLine {
61+
protected bottomBar?: ζinquirer.ui.BottomBar;
62+
63+
protected readonly BottomBar: typeof ζinquirer.ui.BottomBar;
64+
protected readonly rawinput: NodeJS.ReadableStream;
65+
protected readonly rawoutput: NodeJS.WritableStream;
66+
protected readonly colors: Colors;
67+
68+
constructor({ BottomBar, input = process.stdin, output = process.stdout, colors = DEFAULT_COLORS }: BottomBarOutputStrategyOptions) {
69+
this.BottomBar = BottomBar;
70+
this.rawinput = input;
71+
this.rawoutput = output;
72+
this.colors = colors;
73+
}
74+
75+
get stream(): NodeJS.WritableStream {
76+
const bottomBar = this.get();
77+
return bottomBar.log;
78+
}
79+
80+
redrawLine(msg = ''): void {
81+
const bottomBar = this.get();
82+
bottomBar.updateBottomBar(msg);
83+
}
84+
85+
get(): typeof ζinquirer.ui.BottomBar {
86+
if (!this.bottomBar) {
87+
this.bottomBar = new this.BottomBar({ input: this.rawinput, output: this.rawoutput } as any);
88+
89+
try {
90+
// the mute() call appears to be necessary, otherwise when answering
91+
// inquirer prompts upon pressing enter, a copy of the prompt is
92+
// printed to the screen and looks gross
93+
(this.bottomBar as any).rl.output.mute();
94+
} catch (e) {
95+
debug('Error while muting bottomBar output: %o', e);
96+
}
97+
}
98+
99+
return this.bottomBar;
100+
}
101+
102+
open(): void {
103+
this.get();
104+
}
105+
106+
close(): void {
107+
if (this.bottomBar) {
108+
// instantiating inquirer.ui.BottomBar hangs, so when close() is called,
109+
// close BottomBar streams
110+
this.bottomBar.close();
111+
this.bottomBar = undefined;
112+
}
113+
}
114+
115+
createTaskChain() {
116+
const { failure, strong, success } = this.colors;
117+
const chain = new TaskChain({ taskOptions: { tickInterval: 50 } });
118+
119+
this.open();
120+
121+
chain.on('next', task => {
122+
this.open();
123+
124+
task.on('success', () => {
125+
this.stream.write(`${success(ICON_SUCCESS)} ${task.msg} - done!`);
126+
});
127+
128+
task.on('failure', () => {
129+
this.stream.write(`${failure(ICON_FAILURE)} ${task.msg} - failed!`);
130+
});
131+
132+
const spinner = new Spinner();
133+
134+
task.on('tick', () => {
135+
const progress = task.progressRatio ? (task.progressRatio * 100).toFixed(2) : '';
136+
const frame = spinner.frame();
137+
138+
this.redrawLine(`${strong(frame)} ${task.msg}${progress ? ' (' + strong(String(progress) + '%') + ')' : ''} `);
139+
});
140+
141+
task.on('clear', () => {
142+
this.redrawLine('');
143+
});
144+
});
145+
146+
chain.on('end', () => {
147+
this.close();
148+
});
149+
150+
return chain;
151+
}
152+
}

‎packages/@ionic/cli-framework/src/lib/prompts.ts

Lines changed: 5 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import * as Debug from 'debug';
22
import * as ζinquirer from 'inquirer';
33

4-
import { OutputStrategy, RedrawLine } from '../definitions';
54
import { TERMINAL_INFO } from '../utils/terminal';
65

76
const debug = Debug('ionic:cli-framework:lib:prompts');
@@ -66,14 +65,11 @@ export interface PromptQuestionOther extends PromptQuestionBase {
6665
export type PromptQuestion = PromptQuestionConfirm | PromptQuestionCheckbox | PromptQuestionOther;
6766

6867
export interface PromptModule {
69-
readonly output: OutputStrategy & RedrawLine;
68+
readonly _inquirer: ζinquirer.Inquirer;
7069

7170
(question: PromptQuestionConfirm): Promise<PromptValueConfirm>;
7271
(question: PromptQuestionCheckbox): Promise<PromptValueCheckbox>;
7372
(question: PromptQuestionOther): Promise<PromptValueOther>;
74-
75-
open(): void;
76-
close(): void;
7773
}
7874

7975
async function loadInquirer(): Promise<ζinquirer.Inquirer> {
@@ -109,9 +105,9 @@ export interface CreatePromptModuleOptions {
109105
* a 'fallback'.
110106
*/
111107
export async function createPromptModule({ interactive, onFallback }: CreatePromptModuleOptions = {}): Promise<PromptModule> {
112-
const { createPromptModule: createInquirerPromptModule, ui: { BottomBar } } = await loadInquirer();
108+
const inquirer = await loadInquirer();
109+
const { createPromptModule: createInquirerPromptModule } = inquirer;
113110
const promptModule = createInquirerPromptModule();
114-
const manager = new BottomBarManager({ BottomBar });
115111

116112
async function createPrompter(question: PromptQuestionConfirm): Promise<PromptValueConfirm>;
117113
async function createPrompter(question: PromptQuestionCheckbox): Promise<PromptValueCheckbox>;
@@ -161,12 +157,10 @@ export async function createPromptModule({ interactive, onFallback }: CreateProm
161157
}
162158

163159
Object.defineProperties(createPrompter, {
164-
open: { value: () => manager.open() },
165-
close: { value: () => manager.close() },
166-
output: { value: manager },
160+
_inquirer: { value: inquirer },
167161
});
168162

169-
return createPrompter as any;
163+
return createPrompter as any as PromptModule;
170164
}
171165

172166
export function createPromptChoiceSeparator(): ζinquirer.objects.Separator {
@@ -176,63 +170,3 @@ export function createPromptChoiceSeparator(): ζinquirer.objects.Separator {
176170

177171
return new _inquirer.Separator();
178172
}
179-
180-
interface BottomBarManagerOptions {
181-
readonly BottomBar: typeof ζinquirer.ui.BottomBar;
182-
readonly input?: NodeJS.ReadableStream;
183-
readonly output?: NodeJS.WritableStream;
184-
}
185-
186-
class BottomBarManager implements OutputStrategy, RedrawLine {
187-
protected bottomBar?: ζinquirer.ui.BottomBar;
188-
189-
protected readonly BottomBar: typeof ζinquirer.ui.BottomBar;
190-
protected readonly rawinput: NodeJS.ReadableStream;
191-
protected readonly rawoutput: NodeJS.WritableStream;
192-
193-
constructor({ BottomBar, input = process.stdin, output = process.stdout }: BottomBarManagerOptions) {
194-
this.BottomBar = BottomBar;
195-
this.rawinput = input;
196-
this.rawoutput = output;
197-
}
198-
199-
get stream(): NodeJS.WritableStream {
200-
const bottomBar = this.get();
201-
return bottomBar.log;
202-
}
203-
204-
redrawLine(msg = ''): void {
205-
const bottomBar = this.get();
206-
bottomBar.updateBottomBar(msg);
207-
}
208-
209-
get(): typeof ζinquirer.ui.BottomBar {
210-
if (!this.bottomBar) {
211-
this.bottomBar = new this.BottomBar({ input: this.rawinput, output: this.rawoutput } as any);
212-
213-
try {
214-
// the mute() call appears to be necessary, otherwise when answering
215-
// inquirer prompts upon pressing enter, a copy of the prompt is
216-
// printed to the screen and looks gross
217-
(this.bottomBar as any).rl.output.mute();
218-
} catch (e) {
219-
debug('Error while muting bottomBar output: %o', e);
220-
}
221-
}
222-
223-
return this.bottomBar;
224-
}
225-
226-
open(): void {
227-
this.get();
228-
}
229-
230-
close(): void {
231-
if (this.bottomBar) {
232-
// instantiating inquirer.ui.BottomBar hangs, so when close() is called,
233-
// close BottomBar streams
234-
this.bottomBar.close();
235-
this.bottomBar = undefined;
236-
}
237-
}
238-
}

0 commit comments

Comments
 (0)