Merge pull request #4592 from Shopify/jm/remove-ruby/appdev

[Themes] Remove Ruby invocation from the `app dev` command

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

@@ -133,11 +133,6 @@ If you're using the PHP or Ruby app template, then you need to complete the foll
  'Key used to authenticate GraphiQL requests. Should be specified if exposing GraphiQL on a publicly accessible URL. By default, no key is required.',
  env: 'SHOPIFY_FLAG_GRAPHIQL_KEY',
  }),
- legacy: Flags.boolean({
- hidden: true,
- description: 'Use the legacy Ruby implementation for managing theme app extensions.',
- env: 'SHOPIFY_FLAG_LEGACY',
- }),
  }
 
  public static analyticsStopCommand(): string | undefined {
@@ -185,7 +180,6 @@ If you're using the PHP or Ruby app template, then you need to complete the foll
  notify: flags.notify,
  graphiqlPort: flags['graphiql-port'],
  graphiqlKey: flags['graphiql-key'],
- devPreview: !flags.legacy,
  }
 
  const result = await dev(devOptions)

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

@@ -64,7 +64,6 @@ export interface DevOptions {
  notify?: string
  graphiqlPort?: number
  graphiqlKey?: string
- devPreview?: boolean
 }
 
 export async function dev(commandOptions: DevOptions) {

packages/app/src/cli/services/dev/extension/bundler.ts

@@ -13,6 +13,7 @@ import {FSWatcher} from 'chokidar'
 import micromatch from 'micromatch'
 import {deepCompare} from '@shopify/cli-kit/common/object'
 import {Writable} from 'stream'
+import {AsyncResource} from 'async_hooks'
 
 export interface FileWatcherOptions {
  devOptions: ExtensionDevOptions
@@ -170,36 +171,41 @@ Redeploy Paths:
 
  let buildController: AbortController | null
  const allPaths = [...buildPaths, ...configurationPaths]
- const functionRebuildAndRedeployWatcher = chokidar.watch(allPaths, {ignored: '**/*.test.*'}).on('change', (path) => {
- outputDebug(`Extension file at path ${path} changed`, stdout)
- if (buildController) {
- // terminate any existing builds
- buildController.abort()
- }
- buildController = new AbortController()
- const buildSignal = buildController.signal
- const shouldBuild = micromatch.isMatch(path, buildPaths)
-
- reloadAndbuildIfNecessary(extension, shouldBuild, {
- app,
- stdout,
- stderr,
- useTasks: false,
- signal: buildSignal,
- environment: 'development',
- appURL: url,
- })
- .then(({newConfig, previousConfig}) => {
- if (shouldBuild) {
- if (buildSignal.aborted) return
- return onChange()
- }
-
- if (deepCompare(newConfig, previousConfig)) return
- return onChange()
+ const functionRebuildAndRedeployWatcher = chokidar.watch(allPaths, {ignored: '**/*.test.*'}).on(
+ 'change',
+ // We need to bind the execution context to ensure the event handler can access the correct AsyncLocalStorage
+ // See also: https://nodejs.org/api/async_context.html#integrating-asyncresource-with-eventemitter
+ AsyncResource.bind((path) => {
+ outputDebug(`Extension file at path ${path} changed`, stdout)
+ if (buildController) {
+ // terminate any existing builds
+ buildController.abort()
+ }
+ buildController = new AbortController()
+ const buildSignal = buildController.signal
+ const shouldBuild = micromatch.isMatch(path, buildPaths)
+
+ reloadAndbuildIfNecessary(extension, shouldBuild, {
+ app,
+ stdout,
+ stderr,
+ useTasks: false,
+ signal: buildSignal,
+ environment: 'development',
+ appURL: url,
  })
- .catch((error: Error) => onReloadAndBuildError(error))
- })
+ .then(({newConfig, previousConfig}) => {
+ if (shouldBuild) {
+ if (buildSignal.aborted) return
+ return onChange()
+ }
+
+ if (deepCompare(newConfig, previousConfig)) return
+ return onChange()
+ })
+ .catch((error: Error) => onReloadAndBuildError(error))
+ }),
+ )
  listenForAbortOnWatcher(functionRebuildAndRedeployWatcher)
 }
 

packages/app/src/cli/services/dev/processes/setup-dev-processes.test.ts

@@ -5,8 +5,8 @@ import {WebProcess, launchWebProcess} from './web.js'
 import {PreviewableExtensionProcess, launchPreviewableExtensionProcess} from './previewable-extension.js'
 import {launchGraphiQLServer} from './graphiql.js'
 import {pushUpdatesForDraftableExtensions} from './draftable-extension.js'
-import {runThemeAppExtensionsServer} from './theme-app-extension.js'
 import {pushUpdatesForDevSession} from './dev-session.js'
+import {runThemeAppExtensionsServer} from './theme-app-extension.js'
 import {
  testAppAccessConfigExtension,
  testAppConfigExtensions,
@@ -26,12 +26,15 @@ import {describe, test, expect, beforeEach, vi} from 'vitest'
 import {ensureAuthenticatedAdmin, ensureAuthenticatedStorefront} from '@shopify/cli-kit/node/session'
 import {Config} from '@oclif/core'
 import {getEnvironmentVariables} from '@shopify/cli-kit/node/environment'
+import {isStorefrontPasswordProtected} from '@shopify/theme'
+import {fetchTheme} from '@shopify/cli-kit/node/themes/api'
 
 vi.mock('../../context/identifiers.js')
 vi.mock('@shopify/cli-kit/node/session.js')
 vi.mock('../fetch.js')
 vi.mock('@shopify/cli-kit/node/environment')
-
+vi.mock('@shopify/theme')
+vi.mock('@shopify/cli-kit/node/themes/api')
 beforeEach(() => {
  // mocked for draft extensions
  vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue({
@@ -48,6 +51,14 @@ beforeEach(() => {
  })
  vi.mocked(ensureAuthenticatedStorefront).mockResolvedValue('storefront-token')
  vi.mocked(getEnvironmentVariables).mockReturnValue({})
+ vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(false)
+ vi.mocked(fetchTheme).mockResolvedValue({
+ id: 1,
+ name: 'Theme',
+ createdAtRuntime: false,
+ role: 'theme',
+ processing: false,
+ })
 })
 
 describe('setup-dev-processes', () => {
@@ -199,16 +210,19 @@ describe('setup-dev-processes', () => {
  prefix: 'theme-extensions',
  function: runThemeAppExtensionsServer,
  options: {
+ theme: {
+ id: 1,
+ name: 'Theme',
+ createdAtRuntime: false,
+ role: 'theme',
+ processing: false,
+ },
  adminSession: {
  storeFqdn: 'store.myshopify.io',
  token: 'admin-token',
  },
- themeExtensionServerArgs:
- './my-extension --api-key api-key --extension-id extension-id --extension-title theme-extension-name --extension-type THEME_APP_EXTENSION --theme 1'.split(
- ' ',
- ),
- storefrontToken: 'storefront-token',
- developerPlatformClient,
+ themeExtensionDirectory: './my-extension',
+ themeExtensionPort: 9293,
  },
  })
  expect(res.processes[5]).toMatchObject({

packages/app/src/cli/services/dev/processes/setup-dev-processes.ts

@@ -1,9 +1,5 @@
 import {BaseProcess, DevProcessFunction} from './types.js'
 import {PreviewThemeAppExtensionsProcess, setupPreviewThemeAppExtensionsProcess} from './theme-app-extension.js'
-import {
- PreviewThemeAppExtensionsProcess as PreviewThemeAppExtensionsNextProcess,
- setupPreviewThemeAppExtensionsProcess as setupPreviewThemeAppExtensionsProcessNext,
-} from './theme-app-extension-next.js'
 import {PreviewableExtensionProcess, setupPreviewableExtensionsProcess} from './previewable-extension.js'
 import {DraftableExtensionProcess, setupDraftableExtensionsProcess} from './draftable-extension.js'
 import {SendWebhookProcess, setupSendUninstallWebhookProcess} from './uninstall-webhook.js'
@@ -31,7 +27,6 @@ interface ProxyServerProcess extends BaseProcess<{port: number; rules: {[key: st
 type DevProcessDefinition =
  | SendWebhookProcess
  | PreviewThemeAppExtensionsProcess
- | PreviewThemeAppExtensionsNextProcess
  | WebProcess
  | ProxyServerProcess
  | PreviewableExtensionProcess
@@ -141,24 +136,13 @@ export async function setupDevProcesses({
  developerPlatformClient,
  proxyUrl: network.proxyUrl,
  }),
- commandOptions.devPreview
- ? await setupPreviewThemeAppExtensionsProcessNext({
- remoteApp,
- localApp,
- storeFqdn,
- developerPlatformClient,
- theme: commandOptions.theme,
- themeExtensionPort: commandOptions.themeExtensionPort,
- })
- : await setupPreviewThemeAppExtensionsProcess({
- allExtensions: localApp.allExtensions,
- storeFqdn,
- apiKey,
- developerPlatformClient,
- theme: commandOptions.theme,
- themeExtensionPort: commandOptions.themeExtensionPort,
- notify: commandOptions.notify,
- }),
+ await setupPreviewThemeAppExtensionsProcess({
+ remoteApp,
+ localApp,
+ storeFqdn,
+ theme: commandOptions.theme,
+ themeExtensionPort: commandOptions.themeExtensionPort,
+ }),
  setupSendUninstallWebhookProcess({
  webs: localApp.webs,
  backendPort: network.backendPort,