Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove deprecated CLI flags #2603

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Remove deprecated CLI flags #2603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18e3f6c
Remove deprecated --worker cli flag
rbshop Oct 14, 2024
b881013
Remove deprecated --env-branch cli flag
rbshop Oct 14, 2024
d8b1cf1
Added a changeset
rbshop Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gorgeous-paws-move.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/cli-hydrogen': patch
---

Remove deprecated --worker and --env-branch cli flags
75 changes: 1 addition & 74 deletions packages/cli/oclif.manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,26 +340,11 @@
},
"env": {
"description": "Specifies the environment to perform the operation using its handle. Fetch the handle using the `env list` command.",
"exclusive": [
"env-branch"
],
"name": "env",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-branch": {
"deprecated": {
"to": "env",
"message": "--env-branch is deprecated. Use --env instead."
},
"description": "Specifies the environment to perform the operation using its Git branch name.",
"env": "SHOPIFY_HYDROGEN_ENVIRONMENT_BRANCH",
"name": "env-branch",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-file": {
"description": "Path to an environment file to override existing environment variables for the deployment.",
"name": "env-file",
Expand All @@ -369,7 +354,7 @@
"type": "option"
},
"preview": {
"description": "Deploys to the Preview environment. Overrides --env-branch and Git metadata.",
"description": "Deploys to the Preview environment. Overrides Git metadata.",
"name": "preview",
"required": false,
"allowNo": false,
Expand Down Expand Up @@ -594,26 +579,11 @@
},
"env": {
"description": "Specifies the environment to perform the operation using its handle. Fetch the handle using the `env list` command.",
"exclusive": [
"env-branch"
],
"name": "env",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-branch": {
"deprecated": {
"to": "env",
"message": "--env-branch is deprecated. Use --env instead."
},
"description": "Specifies the environment to perform the operation using its Git branch name.",
"env": "SHOPIFY_HYDROGEN_ENVIRONMENT_BRANCH",
"name": "env-branch",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-file": {
"description": "Path to an environment file to override existing environment variables. Defaults to the '.env' located in your project path `--path`.",
"name": "env-file",
Expand Down Expand Up @@ -669,11 +639,6 @@
"allowNo": false,
"type": "boolean"
},
"worker": {
"hidden": true,
"name": "worker",
"type": "boolean"
},
"legacy-runtime": {
"description": "[Classic Remix Compiler] Runs the app in a Node.js sandbox instead of an Oxygen worker.",
"env": "SHOPIFY_HYDROGEN_FLAG_LEGACY_RUNTIME",
Expand Down Expand Up @@ -745,26 +710,11 @@
"flags": {
"env": {
"description": "Specifies the environment to perform the operation using its handle. Fetch the handle using the `env list` command.",
"exclusive": [
"env-branch"
],
"name": "env",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-branch": {
"deprecated": {
"to": "env",
"message": "--env-branch is deprecated. Use --env instead."
},
"description": "Specifies the environment to perform the operation using its Git branch name.",
"env": "SHOPIFY_HYDROGEN_ENVIRONMENT_BRANCH",
"name": "env-branch",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-file": {
"description": "Path to an environment file to override existing environment variables. Defaults to the '.env' located in your project path `--path`.",
"name": "env-file",
Expand Down Expand Up @@ -816,9 +766,6 @@
"flags": {
"env": {
"description": "Specifies the environment to perform the operation using its handle. Fetch the handle using the `env list` command.",
"exclusive": [
"env-branch"
],
"name": "env",
"hasDynamicHelp": false,
"multiple": false,
Expand Down Expand Up @@ -1323,11 +1270,6 @@
"multiple": false,
"type": "option"
},
"worker": {
"hidden": true,
"name": "worker",
"type": "boolean"
},
"legacy-runtime": {
"description": "Runs the app in a Node.js sandbox instead of an Oxygen worker.",
"env": "SHOPIFY_HYDROGEN_FLAG_LEGACY_RUNTIME",
Expand All @@ -1337,26 +1279,11 @@
},
"env": {
"description": "Specifies the environment to perform the operation using its handle. Fetch the handle using the `env list` command.",
"exclusive": [
"env-branch"
],
"name": "env",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-branch": {
"deprecated": {
"to": "env",
"message": "--env-branch is deprecated. Use --env instead."
},
"description": "Specifies the environment to perform the operation using its Git branch name.",
"env": "SHOPIFY_HYDROGEN_ENVIRONMENT_BRANCH",
"name": "env-branch",
"hasDynamicHelp": false,
"multiple": false,
"type": "option"
},
"env-file": {
"description": "Path to an environment file to override existing environment variables. Defaults to the '.env' located in your project path `--path`.",
"name": "env-file",
Expand Down
77 changes: 0 additions & 77 deletions packages/cli/src/commands/hydrogen/deploy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,61 +271,6 @@ describe('deploy', () => {
expect(vi.mocked(renderSuccess)).toHaveBeenCalled;
});

it('calls createDeploy against an environment selected by envBranch', async () => {
vi.mocked(getOxygenDeploymentData).mockResolvedValue({
oxygenDeploymentToken: 'some-encoded-token',
environments: [
{
name: 'Production',
handle: 'production',
branch: 'main',
type: 'PRODUCTION',
},
{name: 'Preview', handle: 'preview', branch: null, type: 'PREVIEW'},
{name: 'Staging', handle: 'staging', branch: 'stage-1', type: 'CUSTOM'},
],
});

await runDeploy({
...deployParams,
envBranch: 'stage-1',
});

expect(vi.mocked(createDeploy)).toHaveBeenCalledWith({
config: {
...expectedConfig,
environmentTag: 'stage-1',
},
hooks: expectedHooks,
logger: deploymentLogger,
});
expect(vi.mocked(renderSuccess)).toHaveBeenCalled;
});

it('calls createDeploy against an envBranch in CI', async () => {
vi.mocked(ciPlatform).mockReturnValue({
isCI: true,
name: 'github',
metadata: {},
});

await runDeploy({
...deployParams,
token: 'some-token',
envBranch: 'stage-1',
});

expect(vi.mocked(createDeploy)).toHaveBeenCalledWith({
config: {
...expectedConfig,
environmentTag: 'stage-1',
},
hooks: expectedHooks,
logger: deploymentLogger,
});
expect(vi.mocked(renderSuccess)).toHaveBeenCalled;
});

it("errors when the env provided doesn't match any environment", async () => {
await expect(
runDeploy({
Expand All @@ -335,15 +280,6 @@ describe('deploy', () => {
).rejects.toThrowError('Environment not found');
});

it("errors when the envBranch provided doesn't match any environment", async () => {
await expect(
runDeploy({
...deployParams,
envBranch: 'fake-branch',
}),
).rejects.toThrowError('Environment not found');
});

it('errors when env is provided while running in CI', async () => {
vi.mocked(ciPlatform).mockReturnValue({
isCI: true,
Expand Down Expand Up @@ -799,19 +735,6 @@ describe('deploy', () => {
message: expect.any(String),
});
});

it('renders a user confirmation on deploy when production environment branch is provided', async () => {
await runDeploy({
...deployParams,
envBranch: 'main',
});

expect(renderConfirmationPrompt).toHaveBeenCalledWith({
confirmationMessage: 'Yes, confirm deploy',
cancellationMessage: 'No, cancel deploy',
message: expect.any(String),
});
});
});

describe('user provides a preview environment', () => {
Expand Down
15 changes: 1 addition & 14 deletions packages/cli/src/commands/hydrogen/deploy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export default class Deploy extends Command {
static flags: any = {
...commonFlags.entry,
...commonFlags.env,
...commonFlags.envBranch,
...overrideFlag(commonFlags.envFile, {
'env-file': {
description:
Expand All @@ -82,7 +81,7 @@ export default class Deploy extends Command {
}),
preview: Flags.boolean({
description:
'Deploys to the Preview environment. Overrides --env-branch and Git metadata.',
'Deploys to the Preview environment. Overrides Git metadata.',
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This previously stated that it Overrides --env-branch, but looking at the original code in this file a little further down it appears that this wasn't actually the case (and I think we should also remove the claim that this overrides the Git metadata too?)

required: false,
default: false,
}),
Expand Down Expand Up @@ -197,7 +196,6 @@ interface OxygenDeploymentOptions {
buildCommand?: string;
defaultEnvironment: boolean;
env?: string;
envBranch?: string;
environmentFile?: string;
force: boolean;
noVerify: boolean;
Expand Down Expand Up @@ -246,7 +244,6 @@ export async function runDeploy(
buildCommand,
defaultEnvironment,
env: envHandle,
envBranch,
environmentFile,
force: forceOnUncommitedChanges,
noVerify,
Expand Down Expand Up @@ -357,10 +354,6 @@ export async function runDeploy(
);
}

if (isCI && envBranch) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added in #2281, with the note:

Future task
env flag will require more effort as it needs to be available on our dependant services
the current fix only patches it for the envBranch flag

Do we still want this feature? Is that future work tracked anywhere?

userProvidedEnvironmentTag = envBranch;
}

if (!isCI) {
deploymentData = await getOxygenDeploymentData({
root,
Expand All @@ -382,11 +375,6 @@ export async function runDeploy(
if (userProvidedEnvironmentTag === null) {
isPreview = true;
}
} else if (envBranch) {
userProvidedEnvironmentTag = findEnvironmentByBranchOrThrow(
deploymentData.environments || [],
envBranch,
).branch;
}
}

Expand All @@ -405,7 +393,6 @@ export async function runDeploy(
!isCI &&
!defaultEnvironment &&
!envHandle &&
!envBranch &&
deploymentData?.environments
) {
if (deploymentData.environments.length > 1) {
Expand Down
5 changes: 0 additions & 5 deletions packages/cli/src/commands/hydrogen/dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ export default class Dev extends Command {
...commonFlags.debug,
...commonFlags.inspectorPort,
...commonFlags.env,
...commonFlags.envBranch,
...commonFlags.envFile,
'disable-version-check': Flags.boolean({
description: 'Skip the version check when running `hydrogen dev`',
Expand All @@ -99,7 +98,6 @@ export default class Dev extends Command {
}),

// For the classic compiler:
worker: deprecated('--worker', {isBoolean: true}),
...overrideFlag(commonFlags.legacyRuntime, {
'legacy-runtime': {
description:
Expand Down Expand Up @@ -160,7 +158,6 @@ type DevOptions = {
disableVirtualRoutes?: boolean;
disableVersionCheck?: boolean;
disableDepsOptimizer?: boolean;
envBranch?: string;
env?: string;
debug?: boolean;
sourcemap?: boolean;
Expand All @@ -180,7 +177,6 @@ export async function runDev({
codegenConfigPath,
disableVirtualRoutes,
disableDepsOptimizer = false,
envBranch,
env: envHandle,
debug = false,
disableVersionCheck = false,
Expand All @@ -207,7 +203,6 @@ export async function runDev({
const envPromise = backgroundPromise.then(({fetchRemote, localVariables}) =>
getAllEnvironmentVariables({
root,
envBranch,
envHandle,
fetchRemote,
localVariables,
Expand Down
20 changes: 0 additions & 20 deletions packages/cli/src/commands/hydrogen/env/pull.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,33 +107,13 @@ describe('pullVariables', () => {
});
});

it('calls getStorefrontEnvVariables when branch is provided', async () => {
await inTemporaryDirectory(async (tmpDir) => {
await runEnvPull({path: tmpDir, envBranch: 'main', envFile});

expect(getStorefrontEnvVariables).toHaveBeenCalledWith(
ADMIN_SESSION,
SHOPIFY_CONFIG.storefront.id,
'production',
);
});
});

it('throws error if handle does not map to any environment', async () => {
await inTemporaryDirectory(async (tmpDir) => {
await expect(
runEnvPull({path: tmpDir, env: 'fake', envFile}),
).rejects.toThrowError('Environment not found');
});
});

it('throws error if branch does not map to any environment', async () => {
await inTemporaryDirectory(async (tmpDir) => {
await expect(
runEnvPull({path: tmpDir, envBranch: 'fake', envFile}),
).rejects.toThrowError('Environment not found');
});
});
});

it('writes environment variables to a .env file by default', async () => {
Expand Down
Loading
Loading