Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions packages/app/src/cli/commands/app/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ export default class Dev extends AppLinkedCommand {
default: false,
exclusive: ['tunnel-url'],
}),
'install-mkcert': Flags.boolean({
description:
'Install and use mkcert to generate localhost certificates when --use-localhost is enabled. Use --no-install-mkcert to fail instead of prompting when certificates are missing.',
env: 'SHOPIFY_FLAG_INSTALL_MKCERT',
allowNo: true,
}),
'localhost-port': Flags.integer({
description: 'Port to use for localhost.',
env: 'SHOPIFY_FLAG_LOCALHOST_PORT',
Expand All @@ -75,6 +81,12 @@ export default class Dev extends AppLinkedCommand {
'The file path or URL. The file path is to a file that you want updated on idle. The URL path is where you want a webhook posted to report on file changes.',
env: 'SHOPIFY_FLAG_NOTIFY',
}),
'convert-transfer-disabled-store': Flags.boolean({
description:
'Convert the selected development store to a transfer-disabled store without prompting. Use --no-convert-transfer-disabled-store to fail instead of prompting.',
env: 'SHOPIFY_FLAG_CONVERT_TRANSFER_DISABLED_STORE',
allowNo: true,
}),
'graphiql-port': Flags.integer({
hidden: true,
description: 'Local port of the GraphiQL development server.',
Expand Down Expand Up @@ -121,6 +133,7 @@ export default class Dev extends AppLinkedCommand {
appContextResult,
storeFqdn: flags.store,
forceReselectStore: flags.reset,
transferDisabledStoreConversion: flags['convert-transfer-disabled-store'],
})

const devOptions: DevOptions = {
Expand All @@ -137,6 +150,7 @@ export default class Dev extends AppLinkedCommand {
notify: flags.notify,
graphiqlPort: flags['graphiql-port'],
graphiqlKey: flags['graphiql-key'],
installMkcert: flags['install-mkcert'],
tunnel: tunnelMode,
}

Expand Down
19 changes: 19 additions & 0 deletions packages/app/src/cli/services/app/config/link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,25 @@ describe('link', () => {
})
})

test('when remote app exists and supports dev sessions then include automatically_update_urls_on_dev = true', async () => {
await inTemporaryDirectory(async (tmp) => {
// Given
const developerPlatformClient = buildDeveloperPlatformClient({supportsDevSessions: true})
const options: LinkOptions = {
directory: tmp,
developerPlatformClient,
}
await mockLoadOpaqueAppWithApp(tmp)
vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient}))

// When
const {configuration} = await link(options)

// Then
expect(configuration.build?.automatically_update_urls_on_dev).toBe(true)
})
})

test('replace arrays content with the remote one', async () => {
await inTemporaryDirectory(async (tmp) => {
// Given
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/app/config/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ function buildOptionsForGeneratedConfigFile(options: {
} = options
const buildOptions = {
...(linkedAppWasNewlyCreated ? {include_config_on_deploy: true} : {}),
...(defaultToUpdateUrlsOnDev && linkedAppWasNewlyCreated ? {automatically_update_urls_on_dev: true} : {}),
...(defaultToUpdateUrlsOnDev ? {automatically_update_urls_on_dev: true} : {}),
...(linkedAppAndClientIdFromFileAreInSync ? existingBuildOptions : {}),
}
if (isEmpty(buildOptions)) {
Expand Down
4 changes: 4 additions & 0 deletions packages/app/src/cli/services/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export interface DevOptions {
notify?: string
graphiqlPort?: number
graphiqlKey?: string
installMkcert?: boolean
}

export async function dev(commandOptions: DevOptions) {
Expand Down Expand Up @@ -152,6 +153,7 @@ async function prepareForDev(commandOptions: DevOptions): Promise<DevConfig> {
tunnel,
tunnelClient,
remoteApp.configuration,
commandOptions.installMkcert,
)
app.webs = webs

Expand Down Expand Up @@ -324,6 +326,7 @@ async function setupNetworkingOptions(
tunnelOptions: TunnelMode,
tunnelClient?: TunnelClient,
remoteAppConfig?: AppConfigurationUsedByCli,
installMkcert?: boolean,
) {
const {backendConfig, frontendConfig} = frontAndBackendConfig(webs)

Expand Down Expand Up @@ -361,6 +364,7 @@ async function setupNetworkingOptions(
if (tunnelOptions.mode === 'use-localhost') {
const {keyContent, certContent, certPath} = await generateCertificate({
appDirectory,
install: installMkcert,
})

reverseProxyCert = {
Expand Down
21 changes: 21 additions & 0 deletions packages/app/src/cli/services/dev/select-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,27 @@ describe('selectStore', async () => {
expect(confirmConversionToTransferDisabledStorePrompt).toHaveBeenCalled()
})

test('converts store without prompting when conversion mode is always', async () => {
// Given
vi.clearAllMocks()
vi.mocked(selectStorePrompt).mockResolvedValueOnce(STORE2)
const developerPlatformClient = testDeveloperPlatformClient({clientName: ClientName.Partners})
const convertToTransferDisabledStore = vi.spyOn(developerPlatformClient, 'convertToTransferDisabledStore')

// When
const got = await selectStore(
{stores: [STORE1, STORE2], hasMorePages: false},
ORG1,
developerPlatformClient,
'always',
)

// Then
expect(got).toEqual(STORE2)
expect(confirmConversionToTransferDisabledStorePrompt).not.toHaveBeenCalled()
expect(convertToTransferDisabledStore).toHaveBeenCalled()
})

test('choosing not to convert to transfer-disabled forces another prompt', async () => {
// Given
vi.mocked(selectStorePrompt).mockResolvedValueOnce(STORE2)
Expand Down
13 changes: 10 additions & 3 deletions packages/app/src/cli/services/dev/select-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {firstPartyDev} from '@shopify/cli-kit/node/context/local'
import {AbortError, BugError, CancelExecution} from '@shopify/cli-kit/node/error'
import {outputSuccess} from '@shopify/cli-kit/node/output'

export type TransferDisabledStoreConversionMode = 'prompt-first' | 'always' | 'never'

/**
* Select store from list or
* If a cachedStoreName is provided, we check if it is valid and return it. If it's not valid, ignore it.
Expand All @@ -29,6 +31,7 @@ export async function selectStore(
storesSearch: Paginateable<{stores: OrganizationStore[]}>,
org: Organization,
developerPlatformClient: DeveloperPlatformClient,
conversionMode: TransferDisabledStoreConversionMode = 'prompt-first',
): Promise<OrganizationStore> {
const showDomainOnPrompt = developerPlatformClient.clientName === ClientName.AppManagement
let onSearchForStoresByName
Expand Down Expand Up @@ -61,7 +64,7 @@ export async function selectStore(
store,
org.id,
developerPlatformClient,
'prompt-first',
conversionMode,
)
while (!storeIsValid) {
// eslint-disable-next-line no-await-in-loop
Expand All @@ -70,7 +73,7 @@ export async function selectStore(
throw new CancelExecution()
}
// eslint-disable-next-line no-await-in-loop
storeIsValid = await convertToTransferDisabledStoreIfNeeded(store, org.id, developerPlatformClient, 'prompt-first')
storeIsValid = await convertToTransferDisabledStoreIfNeeded(store, org.id, developerPlatformClient, conversionMode)
}

return store
Expand Down Expand Up @@ -128,7 +131,7 @@ export async function convertToTransferDisabledStoreIfNeeded(
store: OrganizationStore,
orgId: string,
developerPlatformClient: DeveloperPlatformClient,
conversionMode: 'prompt-first' | 'never',
conversionMode: TransferDisabledStoreConversionMode,
): Promise<boolean> {
if (store.transferDisabled || firstPartyDev()) return true

Expand All @@ -149,6 +152,10 @@ export async function convertToTransferDisabledStoreIfNeeded(
await convertStoreToTransferDisabled(store, orgId, developerPlatformClient)
return true
}
case 'always': {
await convertStoreToTransferDisabled(store, orgId, developerPlatformClient)
return true
}
case 'never': {
throw new AbortError(
'The store you specified is not transfer-disabled',
Expand Down
47 changes: 47 additions & 0 deletions packages/app/src/cli/services/store-context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ describe('storeContext', () => {
{stores: allStores, hasMorePages: false},
mockOrganization,
mockDeveloperPlatformClient,
'prompt-first',
)
expect(result).toEqual(mockStore)
})
Expand All @@ -138,11 +139,57 @@ describe('storeContext', () => {
{stores: allStores, hasMorePages: false},
mockOrganization,
mockDeveloperPlatformClient,
'prompt-first',
)
expect(result).toEqual(mockStore)
})
})

test('converts an explicitly provided store when conversion is requested', async () => {
await inTemporaryDirectory(async (dir) => {
vi.mocked(fetchStore).mockResolvedValue(mockStore)
await prepareAppFolder(mockApp, dir)

await storeContext({
appContextResult,
storeFqdn: 'explicit-store.myshopify.com',
forceReselectStore: false,
transferDisabledStoreConversion: true,
})

expect(convertToTransferDisabledStoreIfNeeded).toHaveBeenCalledWith(
mockStore,
mockOrganization.id,
mockDeveloperPlatformClient,
'always',
)
})
})

test('passes conversion preference when selecting a store', async () => {
await inTemporaryDirectory(async (dir) => {
const appWithoutCachedStore = testAppLinked()
await prepareAppFolder(appWithoutCachedStore, dir)
const allStores = [mockStore]

vi.mocked(mockDeveloperPlatformClient.devStoresForOrg).mockResolvedValue({stores: allStores, hasMorePages: false})
vi.mocked(selectStore).mockResolvedValue(mockStore)

await storeContext({
appContextResult: {...appContextResult, app: appWithoutCachedStore},
forceReselectStore: false,
transferDisabledStoreConversion: true,
})

expect(selectStore).toHaveBeenCalledWith(
{stores: allStores, hasMorePages: false},
mockOrganization,
mockDeveloperPlatformClient,
'always',
)
})
})

test('throws an error when fetchStore fails', async () => {
vi.mocked(fetchStore).mockRejectedValue(new Error('Store not found'))

Expand Down
24 changes: 20 additions & 4 deletions packages/app/src/cli/services/store-context.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import {fetchStore} from './dev/fetch.js'
import {convertToTransferDisabledStoreIfNeeded, selectStore} from './dev/select-store.js'
import {
convertToTransferDisabledStoreIfNeeded,
selectStore,
type TransferDisabledStoreConversionMode,
} from './dev/select-store.js'
import {LoadedAppContextOutput} from './app-context.js'
import {OrganizationStore} from '../models/organization.js'
import {Store} from '../utilities/developer-platform-client.js'
Expand All @@ -20,6 +24,7 @@ interface StoreContextOptions {
forceReselectStore: boolean
storeFqdn?: string
storeTypes?: Store[]
transferDisabledStoreConversion?: boolean
}

/**
Expand All @@ -34,6 +39,7 @@ export async function storeContext({
storeFqdn,
forceReselectStore,
storeTypes = ['APP_DEVELOPMENT'],
transferDisabledStoreConversion,
}: StoreContextOptions): Promise<OrganizationStore> {
const {app, organization, developerPlatformClient} = appContextResult
let selectedStore: OrganizationStore
Expand All @@ -51,17 +57,18 @@ export async function storeContext({

// Check if we're filtering to dev stores only
const isDevStoresOnly = storeTypes.length === 1 && storeTypes[0] === 'APP_DEVELOPMENT'
const conversionMode = transferDisabledStoreConversionMode(transferDisabledStoreConversion, Boolean(storeFqdnToUse))

if (storeFqdnToUse) {
selectedStore = await fetchStore(organization, storeFqdnToUse, developerPlatformClient, storeTypes)
// never automatically convert a store provided via the command line
// Explicit stores keep the previous fail-fast behavior unless conversion is also explicit.
if (isDevStoresOnly) {
await convertToTransferDisabledStoreIfNeeded(selectedStore, organization.id, developerPlatformClient, 'never')
await convertToTransferDisabledStoreIfNeeded(selectedStore, organization.id, developerPlatformClient, conversionMode)
}
} else {
// If no storeFqdn is provided, fetch all stores for the organization and let the user select one.
const allStores = await developerPlatformClient.devStoresForOrg(organization.id)
selectedStore = await selectStore(allStores, organization, developerPlatformClient)
selectedStore = await selectStore(allStores, organization, developerPlatformClient, conversionMode)
}

await logMetadata(selectedStore, forceReselectStore)
Expand All @@ -78,6 +85,15 @@ export async function storeContext({
return selectedStore
}

function transferDisabledStoreConversionMode(
transferDisabledStoreConversion: boolean | undefined,
storeFqdnToUse: boolean,
): TransferDisabledStoreConversionMode {
if (transferDisabledStoreConversion === true) return 'always'
if (transferDisabledStoreConversion === false || storeFqdnToUse) return 'never'
return 'prompt-first'
}

async function logMetadata(selectedStore: OrganizationStore, resetUsed: boolean) {
await metadata.addPublicMetadata(() => ({
cmd_app_reset_used: resetUsed,
Expand Down
35 changes: 35 additions & 0 deletions packages/app/src/cli/utilities/mkcert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,41 @@ describe('mkcert', () => {
expect(downloadGitHubRelease).not.toHaveBeenCalled()
})

testWithTempDir('generates a certificate without prompting when install is true', async ({tempDir}) => {
const appDirectory = tempDir
vi.mocked(generateCertificatePrompt).mockClear()
vi.mocked(exec).mockClear()

vi.mocked(exec).mockImplementation(async () => {
await mkdir(joinPath(appDirectory, '.shopify'))
await writeFile(joinPath(appDirectory, '.shopify', 'localhost-key.pem'), 'key')
await writeFile(joinPath(appDirectory, '.shopify', 'localhost.pem'), 'cert')
})

await generateCertificate({
appDirectory,
install: true,
platform: 'linux',
})

expect(generateCertificatePrompt).not.toHaveBeenCalled()
expect(exec).toHaveBeenCalled()
})

testWithTempDir('fails without prompting when install is false and certificates are missing', async ({tempDir}) => {
vi.mocked(generateCertificatePrompt).mockClear()
vi.mocked(exec).mockClear()
const generatePromise = generateCertificate({
appDirectory: tempDir,
install: false,
platform: 'linux',
})

await expect(generatePromise).rejects.toThrow(AbortError)
expect(generateCertificatePrompt).not.toHaveBeenCalled()
expect(exec).not.toHaveBeenCalled()
})

testWithTempDir('skips certificate generation if the certificate already exists', async ({tempDir}) => {
const appDirectory = tempDir
await mkdir(joinPath(appDirectory, '.shopify'))
Expand Down
4 changes: 3 additions & 1 deletion packages/app/src/cli/utilities/mkcert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ async function downloadMkcertLicense(dotShopifyPath: string): Promise<undefined
interface GenerateCertificateOptions {
appDirectory: string
resetFirst?: boolean
install?: boolean
env?: NodeJS.ProcessEnv
platform?: NodeJS.Platform
arch?: NodeJS.Architecture
Expand All @@ -138,6 +139,7 @@ interface GenerateCertificateOptions {
*/
export async function generateCertificate({
appDirectory,
install,
env = process.env,
platform = process.platform,
arch = process.arch,
Expand All @@ -155,7 +157,7 @@ export async function generateCertificate({
}
}

const shouldGenerate = await generateCertificatePrompt()
const shouldGenerate = install ?? (await generateCertificatePrompt())

if (!shouldGenerate) {
throw new AbortError(`Localhost certificate and key are required at ${relativeCertPath} and ${relativeKeyPath}`)
Expand Down
Loading
Loading