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 --worker CLI flag #2603

Merged
merged 7 commits into from
Oct 24, 2024
Merged

Remove deprecated --worker CLI flag #2603

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 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
fc4331a
update to minor bump
wizardlyhel Oct 23, 2024
56bae64
Revert removal of --env-branch cli flag
rbshop Oct 24, 2024
d212fce
Remove preview metadata description
wizardlyhel Oct 24, 2024
32d9f39
format
wizardlyhel Oct 24, 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
Copy link
Contributor

Choose a reason for hiding this comment

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

@frandiox Should this be a major bump?

Copy link
Contributor

Choose a reason for hiding this comment

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

Technically yes, but since we are releasing under the bundled global CLI, it doesn't matter much. It's not going to be a major bump in the bundled CLI yet, so feel free to do it in a minor here. It's also been announced as deprecated for very long so it's probably OK.

---

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
Contributor 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?)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes - remove --env-branch comment

overrides Git metadata - @frandiox Does hydrogen deploy preview actually overrides git metadata? I don't see its doing that in code either. Running npx shopify hydrogen deploy --preview just creates a preview deploy with no changes to user's repo (which is expected). I think the metadata here is referring to oxygen specific metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reverted the removal of the flag, but I think this update should remain as the code shows that this claim was false anyway

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) {
rbshop marked this conversation as resolved.
Show resolved Hide resolved
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