Skip to content

Commit

Permalink
Drop demo recording (#4841)
Browse files Browse the repository at this point in the history
<!--
  ☝️How to write a good PR title:
  - Prefix it with [Feature] (if applicable)
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Use a draft PR while it’s a work in progress
-->

### WHY are these changes introduced?

Drops the demo recording feature. This was neat, but is unused, contains a lot of code, and has low test coverage.

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.
  Before / after screenshots appreciated for UI changes.
-->

Removes all things related to the demo system

### How to test your changes?

<!--
  Please, provide steps for the reviewer to test your changes locally.
-->

### Post-release steps

<!--
  If changes require post-release steps, for example merging and publishing some documentation changes,
  specify it in this section and add the label "includes-post-release-steps".
  If it doesn't, feel free to remove this section.
-->

### Measuring impact

How do we know this change was effective? Please choose one:

- [ ] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
- [ ] Existing analytics will cater for this addition
- [ ] PR includes analytics changes to measure impact

### Checklist

- [ ] I've considered possible cross-platform impacts (Mac, Linux, Windows)
- [ ] I've considered possible [documentation](https://shopify.dev) changes
  • Loading branch information
shauns authored Nov 13, 2024
1 parent 369875c commit f63a39b
Show file tree
Hide file tree
Showing 22 changed files with 115 additions and 1,740 deletions.
1 change: 0 additions & 1 deletion packages/cli-kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@
"static": [
"@oclif/core",
"./context/utilities.js",
"../../private/node/demo-recorder.js",
"../../private/node/conf-store.js",
"url"
]
Expand Down
171 changes: 0 additions & 171 deletions packages/cli-kit/src/private/node/demo-recorder.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/cli-kit/src/private/node/ui.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function renderOnce(

if (output) {
if (isUnitTest()) collectLog(logLevel, output)
outputWhereAppropriate(logLevel, logger, output, {skipUIEvent: true})
outputWhereAppropriate(logLevel, logger, output)
}

unmount()
Expand Down
4 changes: 1 addition & 3 deletions packages/cli-kit/src/private/node/ui/alert.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {Alert, AlertProps} from './components/Alert.js'
import {renderOnce} from '../ui.js'
import {consoleError, consoleLog, consoleWarn, Logger, LogLevel} from '../../../public/node/output.js'
import {recordUIEvent} from '../demo-recorder.js'
import React from 'react'
import {RenderOptions} from 'ink'

Expand Down Expand Up @@ -35,8 +34,7 @@ export function alert({
renderOptions,
}: AlertOptions) {
// eslint-disable-next-line prefer-rest-params
const {type: alertType, ...eventProps} = arguments[0]
recordUIEvent({type, properties: eventProps})
const {type: alertType, ..._eventProps} = arguments[0]

return renderOnce(
<Alert
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {OutputProcess} from '../../../../public/node/output.js'
import {AbortSignal} from '../../../../public/node/abort.js'
import {addOrUpdateConcurrentUIEventOutput} from '../../demo-recorder.js'
import React, {FunctionComponent, useCallback, useEffect, useMemo, useState} from 'react'
import {Box, Static, Text, TextProps, useApp} from 'ink'
import figures from 'figures'
Expand Down Expand Up @@ -142,7 +141,6 @@ const ConcurrentOutput: FunctionComponent<ConcurrentOutputProps> = ({
const index = addPrefix(prefix, prefixes)

const lines = shouldStripAnsi ? stripAnsi(log).split(/\n/) : log.split(/\n/)
addOrUpdateConcurrentUIEventOutput({prefix, index, output: lines.join('\n')})
setProcessOutput((previousProcessOutput) => [
...previousProcessOutput,
{
Expand Down
2 changes: 0 additions & 2 deletions packages/cli-kit/src/public/node/cli.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {isTruthy} from './context/utilities.js'
import {printEventsJson} from '../../private/node/demo-recorder.js'
import {cacheClear} from '../../private/node/conf-store.js'
import {Flags} from '@oclif/core'
import {fileURLToPath} from 'url'
Expand Down Expand Up @@ -94,7 +93,6 @@ export async function runCLI(options: RunCLIOptions): Promise<void> {

await oclif.default.run(undefined, config)
await oclif.default.flush()
printEventsJson()
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
await errorHandler(error as Error)
Expand Down
4 changes: 1 addition & 3 deletions packages/cli-kit/src/public/node/error-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
import {getEnvironmentData} from '../../private/node/analytics.js'
import {outputDebug, outputInfo} from '../../public/node/output.js'
import {bugsnagApiKey, reportingRateLimit} from '../../private/node/constants.js'
import {printEventsJson} from '../../private/node/demo-recorder.js'
import {CLI_KIT_VERSION} from '../common/version.js'
import {runWithRateLimit} from '../../private/node/conf-store.js'
import {settings, Interfaces} from '@oclif/core'
Expand All @@ -30,14 +29,13 @@ export async function errorHandler(
outputInfo(`✨ ${error.message}`)
}
} else if (error instanceof AbortSilentError) {
printEventsJson()
/* empty */
} else {
return errorMapper(error)
.then((error) => {
return handler(error)
})
.then((mappedError) => {
printEventsJson()
return reportError(mappedError, config)
})
}
Expand Down
2 changes: 0 additions & 2 deletions packages/cli-kit/src/public/node/hooks/prerun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {startAnalytics} from '../../../private/node/analytics.js'
import {outputDebug, outputWarn} from '../../../public/node/output.js'
import {getOutputUpdateCLIReminder} from '../../../public/node/upgrade.js'
import Command from '../../../public/node/base-command.js'
import {initDemoRecorder} from '../../../private/node/demo-recorder.js'
import {runAtMinimumInterval} from '../../../private/node/conf-store.js'
import {Hook} from '@oclif/core'

Expand All @@ -15,7 +14,6 @@ export declare interface CommandContent {
}
// This hook is called before each command run. More info: https://oclif.io/docs/hooks
export const hook: Hook.Prerun = async (options) => {
initDemoRecorder()
const commandContent = parseCommandContent({
id: options.Command.id,
aliases: options.Command.aliases,
Expand Down
14 changes: 1 addition & 13 deletions packages/cli-kit/src/public/node/output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
RawContentToken,
SubHeadingContentToken,
} from '../../private/node/content-tokens.js'
import {recordUIEvent} from '../../private/node/demo-recorder.js'
import {tokenItemToString} from '../../private/node/ui/components/TokenizedText.js'
import stripAnsi from 'strip-ansi'
import {Writable} from 'stream'
Expand Down Expand Up @@ -393,31 +392,20 @@ export function consoleWarn(message: string): void {
console.warn(withOrWithoutStyle(message))
}

interface OutputWhereAppropriateOptions {
skipUIEvent?: boolean
}

/**
* Writes a message to the appropiated logger.
*
* @param logLevel - The log level to use to determine if the message should be output.
* @param logger - The logger to use to output the message.
* @param message - The message to output.
* @param options - Additional options.
*/
export function outputWhereAppropriate(
logLevel: LogLevel,
logger: Logger,
message: string,
options: OutputWhereAppropriateOptions = {skipUIEvent: false},
): void {
export function outputWhereAppropriate(logLevel: LogLevel, logger: Logger, message: string): void {
if (shouldOutput(logLevel)) {
if (logger instanceof Writable) {
logger.write(message)
} else {
logger(message, logLevel)
}
if (!options.skipUIEvent) recordUIEvent({type: 'output', properties: {content: message}})
}
}

Expand Down
2 changes: 0 additions & 2 deletions packages/cli-kit/src/public/node/tree-kill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
/* eslint-disable no-restricted-imports */

import {outputDebug} from './output.js'
import {printEventsJson} from '../../private/node/demo-recorder.js'
import {exec, spawn} from 'child_process'

interface ProcessTree {
Expand Down Expand Up @@ -32,7 +31,6 @@ export function treeKill(
((error?: Error) => {
if (error) outputDebug(`Failed to kill process ${pid}: ${error}`)
})
printEventsJson()
adaptedTreeKill(pid, killSignal, killRoot, after)
}

Expand Down
Loading

0 comments on commit f63a39b

Please sign in to comment.