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.

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

Save dev_store_url under ./shopify #5000

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions packages/app/src/cli/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export const configurationFileNames = {
web: 'shopify.web.toml',
appEnvironments: 'shopify.environments.toml',
lockFile: '.shopify.lock',
hiddenConfig: 'project.json',
hiddenFolder: '.shopify',
} as const

export const dotEnvFileNames = {
Expand Down
1 change: 1 addition & 0 deletions packages/app/src/cli/models/app/app.test-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export function testApp(app: Partial<AppInterface> = {}, schemaType: 'current' |
specifications: app.specifications ?? [],
configSchema: (app.configSchema ?? AppConfigurationSchema) as any,
remoteFlags: app.remoteFlags ?? [],
hiddenConfig: app.hiddenConfig ?? {},
})

if (app.updateDependencies) {
Expand Down
27 changes: 26 additions & 1 deletion packages/app/src/cli/models/app/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
import {Flag} from '../../utilities/developer-platform-client.js'
import {AppAccessSpecIdentifier} from '../extensions/specifications/app_config_app_access.js'
import {WebhookSubscriptionSchema} from '../extensions/specifications/app_config_webhook_schemas/webhook_subscription_schema.js'
import {configurationFileNames} from '../../constants.js'
import {ZodObjectOf, zod} from '@shopify/cli-kit/node/schema'
import {DotEnvFile} from '@shopify/cli-kit/node/dot-env'
import {getDependencies, PackageManager, readAndParsePackageJson} from '@shopify/cli-kit/node/node-package-manager'
import {fileRealPath, findPathUp} from '@shopify/cli-kit/node/fs'
import {fileRealPath, findPathUp, writeFile} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'
import {AbortError} from '@shopify/cli-kit/node/error'
import {normalizeDelimitedString} from '@shopify/cli-kit/common/string'
Expand Down Expand Up @@ -80,6 +81,15 @@
web_directories: zod.array(zod.string()).optional(),
})

/**
* Hidden configuration for an app. Stored inside ./shopify/project.json
* This is a set of values that are needed by the CLI that are not part of the app configuration.
* These are not meant to be git tracked and the user doesn't need to know about their existence.
*/
export interface AppHiddenConfig {
dev_store_url?: string
}

/**
* Utility schema that matches freshly minted or normal, linked, apps.
*/
Expand Down Expand Up @@ -179,6 +189,10 @@
return false
}

export function appHiddenConfigPath(appDirectory: string) {
return joinPath(appDirectory, configurationFileNames.hiddenFolder, configurationFileNames.hiddenConfig)
}

/**
* Get the field names from the configuration that aren't found in the basic built-in app configuration schema.
*/
Expand Down Expand Up @@ -256,6 +270,7 @@
realExtensions: ExtensionInstance[]
draftableExtensions: ExtensionInstance[]
errors?: AppErrors
hiddenConfig: AppHiddenConfig
includeConfigOnDeploy: boolean | undefined
updateDependencies: () => Promise<void>
extensionsForType: (spec: {identifier: string; externalIdentifier: string}) => ExtensionInstance[]
Expand All @@ -274,6 +289,7 @@
creationDefaultOptions(): AppCreationDefaultOptions
manifest: () => Promise<JsonMapType>
removeExtension: (extensionUid: string) => void
updateHiddenConfig: (values: Partial<AppHiddenConfig>) => Promise<void>
}

type AppConstructor<
Expand All @@ -290,6 +306,7 @@
errors?: AppErrors
specifications: ExtensionSpecification[]
remoteFlags?: Flag[]
hiddenConfig: AppHiddenConfig
}

export class App<
Expand All @@ -311,6 +328,7 @@
configSchema: ZodObjectOf<Omit<TConfig, 'path'>>
remoteFlags: Flag[]
realExtensions: ExtensionInstance[]
hiddenConfig: AppHiddenConfig

constructor({
name,
Expand All @@ -326,6 +344,7 @@
specifications,
configSchema,
remoteFlags,
hiddenConfig,
}: AppConstructor<TConfig, TModuleSpec>) {
this.name = name
this.directory = directory
Expand All @@ -340,6 +359,7 @@
this.specifications = specifications
this.configSchema = configSchema ?? AppSchema
this.remoteFlags = remoteFlags ?? []
this.hiddenConfig = hiddenConfig
}

get allExtensions() {
Expand Down Expand Up @@ -388,6 +408,11 @@
this.nodeDependencies = nodeDependencies
}

async updateHiddenConfig(values: Partial<AppHiddenConfig>) {
this.hiddenConfig = {...this.hiddenConfig, ...values}
await writeFile(appHiddenConfigPath(this.directory), JSON.stringify(this.hiddenConfig, null, 2))
}

async preDeployValidation() {
const functionExtensionsWithUiHandle = this.allExtensions.filter(
(ext) => ext.isFunctionExtension && (ext.configuration as unknown as FunctionConfigType).ui?.handle,
Expand Down Expand Up @@ -430,7 +455,7 @@
const frontendConfig = this.webs.find((web) => isWebType(web, WebType.Frontend))
const backendConfig = this.webs.find((web) => isWebType(web, WebType.Backend))

return Boolean(frontendConfig || backendConfig)

Check warning on line 458 in packages/app/src/cli/models/app/app.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/models/app/app.ts#L458

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
}

creationDefaultOptions(): AppCreationDefaultOptions {
Expand Down
19 changes: 18 additions & 1 deletion packages/app/src/cli/models/app/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {
SchemaForConfig,
AppCreationDefaultOptions,
AppLinkedInterface,
appHiddenConfigPath,
AppHiddenConfig,
} from './app.js'
import {showMultipleCLIWarningIfNeeded} from './validation/multi-cli-warning.js'
import {configurationFileNames, dotEnvFileNames} from '../../constants.js'
Expand All @@ -33,7 +35,7 @@ import {WebhooksSchema} from '../extensions/specifications/app_config_webhook_sc
import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js'
import {UIExtensionSchemaType} from '../extensions/specifications/ui_extension.js'
import {deepStrict, zod} from '@shopify/cli-kit/node/schema'
import {fileExists, readFile, glob, findPathUp, fileExistsSync} from '@shopify/cli-kit/node/fs'
import {fileExists, readFile, glob, findPathUp, fileExistsSync, writeFile, mkdir} from '@shopify/cli-kit/node/fs'
import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env'
import {
getDependencies,
Expand Down Expand Up @@ -342,6 +344,8 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
const packageManager = this.previousApp?.packageManager ?? (await getPackageManager(directory))
const usesWorkspaces = this.previousApp?.usesWorkspaces ?? (await appUsesWorkspaces(directory))

const hiddenConfig = await loadHiddenConfig(directory)

if (!this.previousApp) {
await showMultipleCLIWarningIfNeeded(directory, nodeDependencies)
}
Expand All @@ -364,6 +368,7 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
specifications: this.specifications,
configSchema,
remoteFlags: this.remoteFlags,
hiddenConfig,
})

// Show CLI notifications that are targetted for when your app has specific extension types
Expand Down Expand Up @@ -1063,6 +1068,18 @@ async function getAllLinkedConfigClientIds(
return Object.fromEntries(entries)
}

async function loadHiddenConfig(appDirectory: string): Promise<AppHiddenConfig> {
const hiddenConfigPath = appHiddenConfigPath(appDirectory)
if (fileExistsSync(hiddenConfigPath)) {
return JSON.parse(await readFile(hiddenConfigPath, {encoding: 'utf8'}))
} else {
// If the hidden config file doesn't exist, create an empty one.
await mkdir(dirname(hiddenConfigPath))
await writeFile(hiddenConfigPath, '{}')
return {}
}
}

export async function loadAppName(appDirectory: string): Promise<string> {
const packageJSONPath = joinPath(appDirectory, 'package.json')
return (await getPackageName(packageJSONPath)) ?? basename(appDirectory)
Expand Down
5 changes: 3 additions & 2 deletions packages/app/src/cli/services/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@
organization: commandOptions.organization,
})

// Update the dev_store_url in the app configuration if it doesn't match the store domain
if (app.configuration.build?.dev_store_url !== store.shopDomain) {
// If the dev_store_url is set in the app configuration, keep updating it.
// If not, `store-context.ts` will take care of caching it in the hidden config.
if (app.configuration.build?.dev_store_url) {
app.configuration.build = {
...app.configuration.build,
dev_store_url: store.shopDomain,
Expand All @@ -116,10 +117,10 @@
await installAppDependencies(app)
}

const graphiqlPort = commandOptions.graphiqlPort || (await getAvailableTCPPort(ports.graphiql))

Check warning on line 120 in packages/app/src/cli/services/dev.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/dev.ts#L120

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
const {graphiqlKey} = commandOptions

if (graphiqlPort !== (commandOptions.graphiqlPort || ports.graphiql)) {

Check warning on line 123 in packages/app/src/cli/services/dev.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/dev.ts#L123

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
renderWarning({
headline: [
'A random port will be used for GraphiQL because',
Expand Down Expand Up @@ -223,7 +224,7 @@
scopesMessage(getAppScopesArray(localApp.configuration)),
'\n',
'Scopes in Partner Dashboard:',
scopesMessage(remoteAccess?.scopes?.split(',') || []),

Check warning on line 227 in packages/app/src/cli/services/dev.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/dev.ts#L227

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
],
nextSteps,
})
Expand Down Expand Up @@ -301,7 +302,7 @@
...frontEndOptions,
tunnelClient,
}),
getBackendPort() || backendConfig?.configuration.port || getAvailableTCPPort(),

Check warning on line 305 in packages/app/src/cli/services/dev.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/dev.ts#L305

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.

Check warning on line 305 in packages/app/src/cli/services/dev.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/dev.ts#L305

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
getURLs(remoteAppConfig),
])
const proxyUrl = usingLocalhost ? `${frontendUrl}:${proxyPort}` : frontendUrl
Expand Down
5 changes: 4 additions & 1 deletion packages/app/src/cli/services/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ class AppInfo {
['App name', this.remoteApp.title ? {userInput: this.remoteApp.title} : NOT_CONFIGURED_TOKEN],
['Client ID', this.remoteApp.apiKey || NOT_CONFIGURED_TOKEN],
['Access scopes', getAppScopes(this.app.configuration)],
['Dev store', this.app.configuration.build?.dev_store_url ?? NOT_CONFIGURED_TOKEN],
[
'Dev store',
this.app.configuration.build?.dev_store_url ?? this.app.hiddenConfig.dev_store_url ?? NOT_CONFIGURED_TOKEN,
],
['Update URLs', updateUrls],
userAccountInfo,
],
Expand Down
Loading
Loading