Skip to content

Commit dacb80b

Browse files
Trigger auto-upgrade when dev is quit via q or Ctrl+C
The legacy dev UI (Dev.tsx) tore down the process tree with a fixed 2s setTimeout immediately after the user pressed q or hit Ctrl+C. Because oclif only invokes the postrun hook after run() resolves, exiting that early meant the auto-upgrade flow never had a chance to run. Users on the standard `shopify app dev` path therefore stopped receiving CLI auto-upgrades after quitting dev. DevSessionUI.tsx already polled postRunHookHasCompleted() before exiting, but its setInterval was never cleared and the 5s ceiling was too tight for an actual upgrade. This change extracts the shutdown logic into a single helper, waitForPostRunHookAndExit, in cli-kit's postrun module. Both Dev.tsx and DevSessionUI.tsx now call it. The helper: - polls postRunHookHasCompleted() and exits as soon as the hook is done, - caps the wait at a configurable maxWaitMs (default 30s) so a stuck upgrade still terminates the process, - properly clears its interval and guards against double-exit, - delegates to the existing treeKill+process.exit path.
1 parent 95e9788 commit dacb80b

5 files changed

Lines changed: 54 additions & 23 deletions

File tree

packages/app/src/cli/services/dev/ui/components/Dev.test.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ vi.mock('@shopify/cli-kit/node/system', async () => {
2727
vi.mock('../../../context.js')
2828
vi.mock('../../fetch.js')
2929
vi.mock('../../processes/dev-session.js')
30+
vi.mock('@shopify/cli-kit/node/hooks/postrun', async () => {
31+
const actual: any = await vi.importActual('@shopify/cli-kit/node/hooks/postrun')
32+
return {
33+
...actual,
34+
waitForPostRunHookAndExit: vi.fn(),
35+
}
36+
})
3037

3138
const developerPlatformClient = testDeveloperPlatformClient()
3239

packages/app/src/cli/services/dev/ui/components/Dev.tsx

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import {Box, Text, useInput, useStdin} from '@shopify/cli-kit/node/ink'
1010
import {handleCtrlC} from '@shopify/cli-kit/node/ui'
1111
import {openURL} from '@shopify/cli-kit/node/system'
1212
import figures from '@shopify/cli-kit/node/figures'
13-
import {isUnitTest} from '@shopify/cli-kit/node/context/local'
14-
import {treeKill} from '@shopify/cli-kit/node/tree-kill'
13+
import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun'
1514
import {Writable} from 'stream'
1615

1716
export interface DeveloperPreviewController {
@@ -74,12 +73,11 @@ const Dev: FunctionComponent<DevProps> = ({
7473
setIsShuttingDownMessage('Shutting down dev because of an error ...')
7574
} else {
7675
setIsShuttingDownMessage('Shutting down dev ...')
77-
setTimeout(() => {
78-
if (isUnitTest()) return
79-
treeKill(process.pid, 'SIGINT', false, () => {
80-
process.exit(0)
81-
})
82-
}, 2000)
76+
// Wait for the oclif postrun hook (which triggers auto-upgrade) to finish before
77+
// tree-killing the process tree. Exiting earlier — as we used to with a fixed 2s
78+
// timeout — meant `shopify app dev` would never auto-upgrade after the user quit
79+
// with `q` or Ctrl+C.
80+
waitForPostRunHookAndExit()
8381
}
8482
clearInterval(pollingInterval.current)
8583
await developerPreview.disable()

packages/app/src/cli/services/dev/ui/components/DevSessionUI.test.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ vi.mock('@shopify/cli-kit/node/system', async () => {
2424
})
2525
vi.mock('@shopify/cli-kit/node/context/local')
2626
vi.mock('@shopify/cli-kit/node/tree-kill')
27+
vi.mock('@shopify/cli-kit/node/hooks/postrun', async () => {
28+
const actual: any = await vi.importActual('@shopify/cli-kit/node/hooks/postrun')
29+
return {
30+
...actual,
31+
waitForPostRunHookAndExit: vi.fn(),
32+
}
33+
})
2734

2835
const mocks = vi.hoisted(() => {
2936
return {

packages/app/src/cli/services/dev/ui/components/DevSessionUI.tsx

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ import {Box, Text, useInput, useStdin} from '@shopify/cli-kit/node/ink'
1717
import {handleCtrlC} from '@shopify/cli-kit/node/ui'
1818
import {openURL, terminalSupportsHyperlinks} from '@shopify/cli-kit/node/system'
1919
import figures from '@shopify/cli-kit/node/figures'
20-
import {isUnitTest} from '@shopify/cli-kit/node/context/local'
21-
import {treeKill} from '@shopify/cli-kit/node/tree-kill'
22-
import {postRunHookHasCompleted} from '@shopify/cli-kit/node/hooks/postrun'
20+
import {waitForPostRunHookAndExit} from '@shopify/cli-kit/node/hooks/postrun'
2321
import {Writable} from 'stream'
2422

2523
interface DevStatusShortcut extends TabShortcut {
@@ -70,18 +68,10 @@ const DevSessionUI: FunctionComponent<DevSesionUIProps> = ({
7068
setIsShuttingDownMessage('Shutting down dev ...')
7169
await onAbort()
7270
}
73-
if (isUnitTest()) return
74-
75-
// Wait for the post run hook to complete or timeout after 5 seconds.
76-
let totalTime = 0
77-
setInterval(() => {
78-
if (postRunHookHasCompleted() || totalTime > 5000) {
79-
treeKill(process.pid, 'SIGINT', false, () => {
80-
process.exit(0)
81-
})
82-
}
83-
totalTime += 100
84-
}, 100)
71+
// Wait for the oclif postrun hook (which triggers auto-upgrade) to finish before
72+
// tree-killing the process tree, otherwise quitting via `q` or Ctrl+C skips
73+
// auto-upgrade.
74+
waitForPostRunHookAndExit()
8575
})
8676

8777
const errorHandledProcesses = useMemo(() => {

packages/cli-kit/src/public/node/hooks/postrun.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* Postrun hook — uses dynamic imports to avoid loading heavy modules (base-command, analytics)
33
* at module evaluation time. These are only needed after the command has already finished.
44
*/
5+
import {treeKill} from '../tree-kill.js'
56
import {Command, Hook} from '@oclif/core'
67

78
let postRunHookCompleted = false
@@ -15,6 +16,34 @@ export function postRunHookHasCompleted(): boolean {
1516
return postRunHookCompleted
1617
}
1718

19+
/**
20+
* Wait for the postrun hook to finish (so auto-upgrade has a chance to run) and then
21+
* tree-kill the current process tree before exiting.
22+
*
23+
* Long-running interactive commands (e.g. `app dev`) need this when the user terminates
24+
* the command via `q` or Ctrl+C: oclif only runs the postrun hook after `run()` resolves,
25+
* so if we exit too early auto-upgrade never gets to run.
26+
*/
27+
export function waitForPostRunHookAndExit(): void {
28+
const pollIntervalMs = 100
29+
const maxWaitMs = 30000
30+
31+
let elapsed = 0
32+
let terminating = false
33+
const handle = setInterval(() => {
34+
if (terminating) return
35+
if (postRunHookHasCompleted() || elapsed >= maxWaitMs) {
36+
terminating = true
37+
clearInterval(handle)
38+
treeKill(process.pid, 'SIGINT', false, () => {
39+
process.exit(0)
40+
})
41+
return
42+
}
43+
elapsed += pollIntervalMs
44+
}, pollIntervalMs)
45+
}
46+
1847
// This hook is called after each successful command run. More info: https://oclif.io/docs/hooks
1948
export const hook: Hook.Postrun = async ({config, Command}) => {
2049
await detectStopCommand(Command as unknown as typeof Command)

0 commit comments

Comments
 (0)