Merge pull request #4517 from Shopify/enforce-nullable-conf-gets

Force dealing with config get returning undefined

packages/app/src/cli/services/local-storage.test.ts

@@ -83,7 +83,7 @@ describe('setAppInfo', async () => {
         {appId: 'app2', directory: '\\app2\\something', storeFqdn: APP2.storeFqdn, orgId: APP2.orgId},
         storage,
       )
-      const got = storage.get('/app2/something')
+      const got = storage.get('/app2/something')!
 
       // Then
       expect(got.appId).toEqual(APP2.appId)
@@ -114,14 +114,29 @@ describe('clearCurrentConfigFile', async () => {
     await inTemporaryDirectory(async (cwd) => {
       // Given
       const storage = new LocalStorage<AppLocalStorageSchema>({cwd})
-      setCachedAppInfo({directory: APP1_WITH_CONFIG_FILE.directory, configFile: 'shopify.app.staging.toml'})
+      setCachedAppInfo({directory: APP1_WITH_CONFIG_FILE.directory, configFile: 'shopify.app.staging.toml'}, storage)
 
       // When
       clearCurrentConfigFile(APP1_WITH_CONFIG_FILE.directory, storage)
       const got = getCachedAppInfo(APP1_WITH_CONFIG_FILE.directory, storage)
 
       // Then
-      expect(got?.configFile).toBeUndefined()
+      expect(got).toBeDefined()
+      expect(got!.configFile).toBeUndefined()
+    })
+  })
+
+  test('no-ops if there is no current config in local storage', async () => {
+    await inTemporaryDirectory(async (cwd) => {
+      // Given
+      const storage = new LocalStorage<AppLocalStorageSchema>({cwd})
+
+      // When
+      clearCurrentConfigFile(APP1_WITH_CONFIG_FILE.directory, storage)
+      const got = getCachedAppInfo(APP1_WITH_CONFIG_FILE.directory, storage)
+
+      // Then
+      expect(got).toBeUndefined()
     })
   })
 })

packages/app/src/cli/services/local-storage.ts

@@ -74,6 +74,10 @@ export function clearCurrentConfigFile(
 ): void {
   const normalized = normalizePath(directory)
   const savedApp = config.get(normalized)
+  if (!savedApp) {
+    return
+  }
+
   config.set(normalized, {
     ...savedApp,
     configFile: undefined,

packages/cli-kit/src/public/node/local-storage.ts

@@ -18,7 +18,7 @@ export class LocalStorage<T extends {[key: string]: any}> {
    * @param key - The key to get.
    * @returns The value.
    */
-  get<TKey extends keyof T>(key: TKey): T[TKey] {
+  get<TKey extends keyof T>(key: TKey): T[TKey] | undefined {
     return this.config.get(key)
   }
 

packages/plugin-did-you-mean/src/services/conf.test.ts

@@ -4,6 +4,13 @@ import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs'
 import {LocalStorage} from '@shopify/cli-kit/node/local-storage'
 
 describe('did-you-mean config', async () => {
+  test('isAutocorrectEnabled returns false if no cached value exists', async () => {
+    await inTemporaryDirectory(async (cwd) => {
+      const conf = new LocalStorage<ConfigSchema>({cwd})
+      expect(isAutocorrectEnabled(conf)).toBe(false)
+    })
+  })
+
   test('isAutocorrectEnabled returns true if cached value is true', async () => {
     await inTemporaryDirectory(async (cwd) => {
       // Given

packages/plugin-did-you-mean/src/services/conf.ts

@@ -1,7 +1,7 @@
 import {LocalStorage} from '@shopify/cli-kit/node/local-storage'
 
 export function isAutocorrectEnabled(conf: LocalStorage<ConfigSchema> = getConfig()): boolean {
-  return conf.get('autocorrectEnabled')
+  return Boolean(conf.get('autocorrectEnabled'))
 }
 
 export function setAutocorrect(value: boolean, conf: LocalStorage<ConfigSchema> = getConfig()) {

packages/theme/src/cli/services/local-storage.ts

@@ -1,3 +1,4 @@
+import {BugError} from '@shopify/cli-kit/node/error'
 import {LocalStorage} from '@shopify/cli-kit/node/local-storage'
 import {outputDebug, outputContent} from '@shopify/cli-kit/node/output'
 
@@ -64,48 +65,63 @@ export function setThemeStore(store: string) {
 
 export function getDevelopmentTheme(): string | undefined {
   outputDebug(outputContent`Getting development theme...`)
-  return developmentThemeLocalStorage().get(getThemeStore())
+  return developmentThemeLocalStorage().get(requireThemeStore())
 }
 
 export function setDevelopmentTheme(theme: string): void {
   outputDebug(outputContent`Setting development theme...`)
-  developmentThemeLocalStorage().set(getThemeStore(), theme)
+  developmentThemeLocalStorage().set(requireThemeStore(), theme)
 }
 
 export function removeDevelopmentTheme(): void {
   outputDebug(outputContent`Removing development theme...`)
-  developmentThemeLocalStorage().delete(getThemeStore())
+  developmentThemeLocalStorage().delete(requireThemeStore())
 }
 
 export function getREPLTheme(): string | undefined {
   outputDebug(outputContent`Getting REPL theme...`)
-  return replThemeLocalStorage().get(getThemeStore())
+  return replThemeLocalStorage().get(requireThemeStore())
 }
 
 export function setREPLTheme(theme: string): void {
   outputDebug(outputContent`Setting REPL theme to ${theme}...`)
-  replThemeLocalStorage().set(getThemeStore(), theme)
+  replThemeLocalStorage().set(requireThemeStore(), theme)
 }
 
 export function removeREPLTheme(): void {
   outputDebug(outputContent`Removing REPL theme...`)
-  replThemeLocalStorage().delete(getThemeStore())
+  replThemeLocalStorage().delete(requireThemeStore())
 }
 
 export function getStorefrontPassword(): string | undefined {
-  const themeStore = getThemeStore()
+  const themeStore = requireThemeStore()
   outputDebug(outputContent`Getting storefront password for shop ${themeStore}...`)
-  return themeStorePasswordStorage().get(getThemeStore())
+  return themeStorePasswordStorage().get(themeStore)
 }
 
 export function setStorefrontPassword(password: string): void {
-  const themeStore = getThemeStore()
+  const themeStore = requireThemeStore()
   outputDebug(outputContent`Setting storefront password for shop ${themeStore}...`)
   themeStorePasswordStorage().set(themeStore, password)
 }
 
 export function removeStorefrontPassword(): void {
-  const themeStore = getThemeStore()
+  const themeStore = requireThemeStore()
   outputDebug(outputContent`Removing storefront password for ${themeStore}...`)
   themeStorePasswordStorage().delete(themeStore)
 }
+
+function requireThemeStore(): string {
+  const themeStore = getThemeStore()
+  if (!themeStore) {
+    throw new BugError(
+      'Theme store is not set. This indicates an unexpected issue with the CLI. Please report this to the Shopify CLI team.',
+      [
+        'It may be possible to recover by running',
+        {command: 'shopify theme list --store <store>'},
+        '(setting the store flag to the store you wish to use) and then running the command again.',
+      ],
+    )
+  }
+  return themeStore
+}

packages/theme/src/cli/services/pull.test.ts

@@ -1,4 +1,5 @@
 import {isEmptyDir, pull, PullFlags} from './pull.js'
+import {setThemeStore} from './local-storage.js'
 import {findOrSelectTheme} from '../utilities/theme-selector.js'
 import {ensureThemeStore} from '../utilities/theme-store.js'
 import {DevelopmentThemeManager} from '../utilities/development-theme-manager.js'
@@ -39,7 +40,11 @@ describe('pull', () => {
   const fetchDevelopmentThemeSpy = vi.spyOn(DevelopmentThemeManager.prototype, 'fetch')
 
   beforeEach(() => {
-    vi.mocked(ensureThemeStore).mockReturnValue('example.myshopify.com')
+    vi.mocked(ensureThemeStore).mockImplementation(() => {
+      const themeStore = 'example.myshopify.com'
+      setThemeStore(themeStore)
+      return themeStore
+    })
     vi.mocked(ensureAuthenticatedThemes).mockResolvedValue(adminSession)
     vi.mocked(mountThemeFileSystem).mockReturnValue(localThemeFileSystem)
     vi.mocked(fetchChecksums).mockResolvedValue([])