-
Notifications
You must be signed in to change notification settings - Fork 130
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Drop support for local CLI switchover (#4842)
### WHY are these changes introduced? Drops support for the local-mode CLI switchover. This never got full release and we've changed our approach pretty substantially since then. Also: Refactors the CLI launching mechanism to improve testability and reduce side effects by: - Extracting core CLI launching logic into a dedicated module - Making environment variables and process arguments injectable - Adding comprehensive test coverage ### WHAT is this pull request doing? - Creates new `cli-launcher.ts` module to handle core CLI launching functionality - Refactors `cli.ts` to use dependency injection for better testing - Adds extensive test coverage for CLI launching and environment setup - Removes unused code related to local CLI detection - Makes environment variables and process arguments configurable ### How to test your changes? 1. Run the CLI normally to verify basic functionality works 2. Try launching with various flags like `--verbose` and `--no-color` ### Measuring impact - [x] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix ### Checklist - [x] I've considered possible cross-platform impacts (Mac, Linux, Windows) - [x] I've considered possible documentation changes
- Loading branch information
Showing
6 changed files
with
237 additions
and
121 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import {launchCLI} from './cli-launcher.js' | ||
| import {describe, expect, test} from 'vitest' | ||
|
|
||
| describe('launchCLI', () => { | ||
| test('launches the CLI', async () => { | ||
| const originalStdoutWrite = process.stdout.write | ||
| const outputs: string[] = [] | ||
| process.stdout.write = (str) => { | ||
| outputs.push(str as any) | ||
| return true | ||
| } | ||
|
|
||
| await launchCLI({moduleURL: import.meta.url, argv: ['--help']}) | ||
| expect(outputs.join('\n')).toContain( | ||
| 'A set of utilities, interfaces, and models that are common across all the platform features', | ||
| ) | ||
| // eslint-disable-next-line require-atomic-updates | ||
| process.stdout.write = originalStdoutWrite | ||
| }) | ||
|
|
||
| test('fails if args are invalid', async () => { | ||
| await expect(launchCLI({moduleURL: import.meta.url, argv: ['this', 'is', 'invalid']})).rejects.toThrow() | ||
| }) | ||
| }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import {fileURLToPath} from 'node:url' | ||
|
|
||
| interface Options { | ||
| moduleURL: string | ||
| argv?: string[] | ||
| } | ||
|
|
||
| /** | ||
| * Launches the CLI through our custom OCLIF loader. | ||
| * | ||
| * @param options - Options. | ||
| * @returns A promise that resolves when the CLI has been launched. | ||
| */ | ||
| export async function launchCLI(options: Options): Promise<void> { | ||
| const {errorHandler} = await import('./error-handler.js') | ||
| const {isDevelopment} = await import('./context/local.js') | ||
| const oclif = await import('@oclif/core') | ||
| const {ShopifyConfig} = await import('./custom-oclif-loader.js') | ||
|
|
||
| if (isDevelopment()) { | ||
| oclif.default.settings.debug = true | ||
| } | ||
|
|
||
| try { | ||
| // Use a custom OCLIF config to customize the behavior of the CLI | ||
| const config = new ShopifyConfig({root: fileURLToPath(options.moduleURL)}) | ||
| await config.load() | ||
|
|
||
| await oclif.default.run(options.argv, config) | ||
| await oclif.default.flush() | ||
| // eslint-disable-next-line no-catch-all/no-catch-all | ||
| } catch (error) { | ||
| await errorHandler(error as Error) | ||
| return oclif.default.Errors.handle(error as Error) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import {clearCache, runCLI, runCreateCLI} from './cli.js' | ||
| import {findUpAndReadPackageJson} from './node-package-manager.js' | ||
| import {mockAndCaptureOutput} from './testing/output.js' | ||
| import {cacheClear} from '../../private/node/conf-store.js' | ||
| import {describe, expect, test, vi} from 'vitest' | ||
|
|
||
| vi.mock('./node-package-manager.js') | ||
| vi.mock('../../private/node/conf-store.js') | ||
|
|
||
| describe('cli', () => { | ||
| test('prepares to run the CLI', async () => { | ||
| const launchCLI = vi.fn() | ||
| await runCLI({moduleURL: 'test', development: false}, launchCLI) | ||
| expect(launchCLI).toHaveBeenCalledWith({moduleURL: 'test'}) | ||
| }) | ||
|
|
||
| test('triggers no colour mode based on --no-color flag', async () => { | ||
| const launchCLI = vi.fn() | ||
| const env = {} as any | ||
| await runCLI({moduleURL: 'test', development: false}, launchCLI, ['--no-color'], env) | ||
| expect(env.FORCE_COLOR).toBe('0') | ||
| }) | ||
|
|
||
| test('triggers no colour mode based on NO_COLOR environment variable', async () => { | ||
| const launchCLI = vi.fn() | ||
| const env = {NO_COLOR: 'TRUE'} as any | ||
| await runCLI({moduleURL: 'test', development: false}, launchCLI, [], env) | ||
| expect(env.FORCE_COLOR).toBe('0') | ||
| }) | ||
|
|
||
| test('triggers no colour mode based on SHOPIFY_FLAG_NO_COLOR environment variable', async () => { | ||
| const launchCLI = vi.fn() | ||
| const env = {SHOPIFY_FLAG_NO_COLOR: 'TRUE'} as any | ||
| await runCLI({moduleURL: 'test', development: false}, launchCLI, [], env) | ||
| expect(env.FORCE_COLOR).toBe('0') | ||
| }) | ||
|
|
||
| test('triggers no colour mode based on TERM environment variable', async () => { | ||
| const launchCLI = vi.fn() | ||
| const env = {TERM: 'dumb'} as any | ||
| await runCLI({moduleURL: 'test', development: false}, launchCLI, [], env) | ||
| expect(env.FORCE_COLOR).toBe('0') | ||
| }) | ||
|
|
||
| test('triggers DEBUG based on --verbose flag', async () => { | ||
| const launchCLI = vi.fn() | ||
| const env = {} as any | ||
| await runCLI({moduleURL: 'test', development: false}, launchCLI, ['--verbose'], env) | ||
| expect(env.DEBUG).toBe('*') | ||
| }) | ||
|
|
||
| test('triggers SHOPIFY_CLI_ENV based on running in development mode', async () => { | ||
| const launchCLI = vi.fn() | ||
| const env = {} as any | ||
| await runCLI({moduleURL: 'test', development: true}, launchCLI, [], env) | ||
| expect(env.SHOPIFY_CLI_ENV).toBe('development') | ||
| }) | ||
|
|
||
| test('warns if running an old Node version', async () => { | ||
| const launchCLI = vi.fn() | ||
| const outputMock = mockAndCaptureOutput() | ||
| await runCLI({moduleURL: 'test', development: false}, launchCLI, [], {}, {node: '17.9'} as any) | ||
| expect(outputMock.output()).toMatchInlineSnapshot(` | ||
| "╭─ warning ────────────────────────────────────────────────────────────────────╮ | ||
| │ │ | ||
| │ Upgrade to a supported Node version now. │ | ||
| │ │ | ||
| │ Node 17 has reached end-of-life and poses security risks. When you upgrade │ | ||
| │ to a supported version [1], you'll be able to use Shopify CLI without │ | ||
| │ interruption. │ | ||
| │ │ | ||
| ╰──────────────────────────────────────────────────────────────────────────────╯ | ||
| [1] https://nodejs.dev/en/about/previous-releases | ||
| " | ||
| `) | ||
| }) | ||
|
|
||
| test('changes process.argv when running create-x', async () => { | ||
| const launchCLI = vi.fn() | ||
| const argv = ['node', 'packages/create-app/bin/run.js', '--verbose'] | ||
| vi.mocked(findUpAndReadPackageJson).mockResolvedValue({content: {name: '@shopify/create-app'}} as any) | ||
| await runCreateCLI({moduleURL: import.meta.url, development: false}, launchCLI, argv, {}) | ||
| expect(argv).toMatchInlineSnapshot(` | ||
| [ | ||
| "node", | ||
| "packages/create-app/bin/run.js", | ||
| "init", | ||
| "--verbose", | ||
| ] | ||
| `) | ||
| }) | ||
|
|
||
| test('leaves process.argv unchanged if init is already present', async () => { | ||
| const launchCLI = vi.fn() | ||
| const argv = ['node', 'packages/create-app/bin/run.js', 'init', '--verbose'] | ||
| vi.mocked(findUpAndReadPackageJson).mockResolvedValue({content: {name: '@shopify/create-app'}} as any) | ||
| await runCreateCLI({moduleURL: import.meta.url, development: false}, launchCLI, argv, {}) | ||
| expect(argv).toMatchInlineSnapshot(` | ||
| [ | ||
| "node", | ||
| "packages/create-app/bin/run.js", | ||
| "init", | ||
| "--verbose", | ||
| ] | ||
| `) | ||
| }) | ||
| }) | ||
|
|
||
| describe('clearCache', () => { | ||
| test('clears the cache', () => { | ||
| clearCache() | ||
| expect(cacheClear).toHaveBeenCalled() | ||
| }) | ||
| }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.