CLI triggers own deprecation warning

[sambostock]

Problem

We are triggering our own deprecation warning.

• --api-key has been renamed to --client-id
• If --api-key is not provided, we check for $SHOPIFY_API_KEY, and set it as --api-key
• If --api-key is set, we emit a deprecation warning

cli/packages/app/src/cli/commands/app/dev.ts

Lines 144 to 151 in 2450a80

	if (!flags['api-key']) {
	if (process.env.SHOPIFY_API_KEY) {
	flags['api-key'] = process.env.SHOPIFY_API_KEY
	}
	}
	if (flags['api-key']) {
	await showApiKeyDeprecationWarning()
	}

Solution

• 🅰 : We continue to support $SHOPIFY_API_KEY, but stop logging a deprecation warning when we get the key from there rather than --api-key directly.
• 🅱 : We also deprecate $SHOPIFY_API_KEY in favour of $SHOPIFY_CLIENT_ID and clarify deprecation warning.

Tip

The flags config implies it will automatically check $SHOPIFY_FLAG_APP_API_KEY/$SHOPIFY_FLAG_CLIENT_ID environment variables, if --api-key/--client-id are omitted. Maybe we should use those? Although for some reason, we have both those flags as hidden.

[isaacroldan]

Hi @sambostock! We want to show the warning for any usage of api-key or the equivalent env vars: SHOPIFY_API_KEY, SHOPIFY_FLAG_API_KEY and SHOPIFY_FLAG_APP_API_KEY

Specifically, SHOPIFY_API_KEY is only used for dev and deploy commands.

The only valid client-id flag for any app command is SHOPIFY_FLAG_CLIENT_ID. (this one is not hidden)

[sambostock]

@isaacroldan consolidating on the single flag (or corresponding environment variable) makes sense, but the deprecation warning is confusing as it specifically mentions the flags when the user might not be using them. Should we clarify that?

[isaacroldan]

Definitely, that makes sense. Do you want to open a PR to fix this? We always welcome contributions from other teams! :)