fix(themes): several fixes for themes:preview and themes:migrate

packages/zcli-themes/src/commands/themes/migrate.ts

@@ -18,9 +18,9 @@ export default class Migrate extends Command {
   static strict = false
 
   async run () {
-    const { flags, argv: [themeDirectory] } = await this.parse(Migrate)
+    const { argv: [themeDirectory] } = await this.parse(Migrate)
     const themePath = path.resolve(themeDirectory)
 
-    await migrate(themePath, flags)
+    await migrate(themePath)
   }
 }

packages/zcli-themes/src/commands/themes/preview.ts

@@ -99,7 +99,7 @@ export default class Preview extends Command {
       `${themePath}/style.css`
     ]
 
-    const watcher = chokidar.watch(monitoredPaths).on('change', async (path) => {
+    const handleThemeChange = async (path: string) => {
       this.log(chalk.bold('Change'), path)
       try {
         await preview(themePath, flags)
@@ -111,7 +111,12 @@ export default class Preview extends Command {
       } catch (e) {
         this.error(e as Error, { exit: false })
       }
-    })
+    }
+
+    const watcher = chokidar.watch(monitoredPaths, { ignoreInitial: true })
+      .on('add', handleThemeChange)
+      .on('change', handleThemeChange)
+      .on('unlink', handleThemeChange)
 
     return {
       close: () => {

packages/zcli-themes/src/lib/getAssets.test.ts

@@ -39,6 +39,30 @@ describe('getAssets', () => {
     ])
   })
 
+  it('returns basenames as urls when flags are not provided', () => {
+    const existsSyncStub = sinon.stub(fs, 'existsSync')
+    const readdirSyncStub = sinon.stub(fs, 'readdirSync')
+
+    existsSyncStub
+      .withArgs('theme/path/assets')
+      .returns(true)
+
+    readdirSyncStub.returns(['foo.png', 'bar.png'] as any)
+
+    const assets = getAssets('theme/path')
+
+    expect(assets).to.deep.equal([
+      [
+        { base: 'foo.png', dir: '', ext: '.png', name: 'foo', root: '' },
+        'foo.png'
+      ],
+      [
+        { base: 'bar.png', dir: '', ext: '.png', name: 'bar', root: '' },
+        'bar.png'
+      ]
+    ])
+  })
+
   it('throws an error when an asset has illegal characters in its name', () => {
     const existsSyncStub = sinon.stub(fs, 'existsSync')
     const readdirSyncStub = sinon.stub(fs, 'readdirSync')

packages/zcli-themes/src/lib/getAssets.ts

@@ -4,7 +4,7 @@ import * as fs from 'fs'
 import * as path from 'path'
 import { getLocalServerBaseUrl } from './getLocalServerBaseUrl'
 
-export default function getAssets (themePath: string, flags: Flags): [path.ParsedPath, string][] {
+export default function getAssets (themePath: string, flags?: Flags): [path.ParsedPath, string][] {
   const assetsPath = `${themePath}/assets`
   const filenames = fs.existsSync(assetsPath) ? fs.readdirSync(assetsPath) : []
   const assets: [path.ParsedPath, string][] = []
@@ -18,7 +18,8 @@ export default function getAssets (themePath: string, flags: Flags): [path.Parse
       )
     }
     if (!name.startsWith('.')) {
-      assets.push([parsedPath, `${getLocalServerBaseUrl(flags)}/guide/assets/${filename}`])
+      const url = flags ? `${getLocalServerBaseUrl(flags)}/guide/assets/${filename}` : filename
+      assets.push([parsedPath, url])
     }
   })
 

packages/zcli-themes/src/lib/getVariables.test.ts

@@ -40,6 +40,23 @@ describe('getVariables', () => {
     ])
   })
 
+  it('uses the matched filename as the value when flags are not provided', () => {
+    const existsSyncStub = sinon.stub(fs, 'existsSync')
+    const readdirSyncStub = sinon.stub(fs, 'readdirSync')
+
+    existsSyncStub
+      .withArgs('theme/path/settings')
+      .returns(true)
+
+    readdirSyncStub.returns(['logo.png', 'favicon.png'] as any)
+
+    expect(getVariables('theme/path', settings)).to.deep.equal([
+      { identifier: 'color', type: 'color', value: '#999' },
+      { identifier: 'logo', type: 'file', value: 'logo.png' },
+      { identifier: 'favicon', type: 'file', value: 'favicon.png' }
+    ])
+  })
+
   it('throws an error when it doesn\'t find an asset within the settings folder for a variable of type "file"', () => {
     const existsSyncStub = sinon.stub(fs, 'existsSync')
     const readdirSyncStub = sinon.stub(fs, 'readdirSync')

packages/zcli-themes/src/lib/getVariables.ts

@@ -4,7 +4,7 @@ import * as path from 'path'
 import { CLIError } from '@oclif/core/lib/errors'
 import { getLocalServerBaseUrl } from './getLocalServerBaseUrl'
 
-export default function getVariables (themePath: string, settings: Setting[], flags: Flags): Variable[] {
+export default function getVariables (themePath: string, settings: Setting[], flags?: Flags): Variable[] {
   const settingsPath = `${themePath}/settings`
   const filenames = fs.existsSync(settingsPath) ? fs.readdirSync(settingsPath) : []
 
@@ -18,7 +18,7 @@ export default function getVariables (themePath: string, settings: Setting[], fl
             `The setting "${variable.identifier}" of type "file" does not have a matching file within the "settings" folder`
           )
         }
-        variable.value = file && `${getLocalServerBaseUrl(flags)}/guide/settings/${file}`
+        variable.value = flags ? `${getLocalServerBaseUrl(flags)}/guide/settings/${file}` : file
       }
       return variable
     })

packages/zcli-themes/src/lib/migrate.test.ts

@@ -27,13 +27,6 @@ const manifest = {
   ]
 }
 
-const flags = {
-  bind: 'localhost',
-  port: 1000,
-  logs: true,
-  livereload: false
-}
-
 describe('migrate', () => {
   beforeEach(() => {
     sinon.restore()
@@ -56,16 +49,16 @@ describe('migrate', () => {
       'custom_pages/faq': '<h1>FAQ</h1>'
     })
 
-    getVariablesStub.withArgs('theme/path', manifest.settings, flags).returns([
+    getVariablesStub.withArgs('theme/path', manifest.settings).returns([
       { identifier: 'color', type: 'color', value: '#999' },
       {
         identifier: 'logo',
         type: 'file',
-        value: 'http://localhost:1000/guide/settings/logo.png'
+        value: 'logo.png'
       }
     ])
 
-    getAssetsStub.withArgs('theme/path', flags).returns([
+    getAssetsStub.withArgs('theme/path').returns([
       [
         {
           base: 'background.png',
@@ -74,7 +67,7 @@ describe('migrate', () => {
           name: 'background',
           root: ''
         },
-        'http://localhost:1000/guide/assets/background.png'
+        'background.png'
       ]
     ])
 
@@ -98,7 +91,7 @@ describe('migrate', () => {
       }) as axios.AxiosPromise
     )
 
-    await migrate('theme/path', flags)
+    await migrate('theme/path')
 
     expect(
       requestStub.calledWith(
@@ -114,12 +107,11 @@ describe('migrate', () => {
               'article_pages/product_updates': '<h1>Product updates</h1>',
               'custom_pages/faq': '<h1>FAQ</h1>',
               assets: {
-                'background.png':
-                  'http://localhost:1000/guide/assets/background.png'
+                'background.png': 'background.png'
               },
               variables: {
                 color: '#999',
-                logo: 'http://localhost:1000/guide/settings/logo.png'
+                logo: 'logo.png'
               },
               metadata: { api_version: 1 }
             }
@@ -178,7 +170,7 @@ describe('migrate', () => {
     const errorStub = sinon.stub(errors, 'error').callThrough()
 
     try {
-      await migrate('theme/path', flags)
+      await migrate('theme/path')
     } catch {
       const [call] = errorStub.getCalls()
       const [error] = call.args
@@ -206,7 +198,7 @@ describe('migrate', () => {
     const errorStub = sinon.stub(errors, 'error').callThrough()
 
     try {
-      await migrate('theme/path', flags)
+      await migrate('theme/path')
     } catch {
       const [call] = errorStub.getCalls()
       const [error] = call.args
@@ -230,7 +222,7 @@ describe('migrate', () => {
     const errorStub = sinon.stub(errors, 'error').callThrough()
 
     try {
-      await migrate('theme/path', flags)
+      await migrate('theme/path')
     } catch {
       const [call] = errorStub.getCalls()
       const [error] = call.args
@@ -250,7 +242,7 @@ describe('migrate', () => {
     const errorStub = sinon.stub(errors, 'error').callThrough()
 
     try {
-      await migrate('theme/path', flags)
+      await migrate('theme/path')
     } catch {
       const [call] = errorStub.getCalls()
       const [error] = call.args

packages/zcli-themes/src/lib/migrate.ts

@@ -1,4 +1,4 @@
-import type { Flags, ValidationErrors } from '../types'
+import type { ValidationErrors } from '../types'
 import getManifest from './getManifest'
 import getTemplates from './getTemplates'
 import getVariables from './getVariables'
@@ -14,11 +14,11 @@ import rewriteAssets from './rewriteAssets'
 import handleTemplateError from './handleTemplateError'
 import parseAxiosError from './parseAxiosError'
 
-export default async function migrate (themePath: string, flags: Flags): Promise<string | void> {
+export default async function migrate (themePath: string): Promise<string | void> {
   const manifest = getManifest(themePath)
   const templates = getTemplates(themePath)
-  const variables = getVariables(themePath, manifest.settings, flags)
-  const assets = getAssets(themePath, flags)
+  const variables = getVariables(themePath, manifest.settings)
+  const assets = getAssets(themePath)
 
   const variablesPayload = variables.reduce((payload, variable) => ({
     ...payload,

packages/zcli-themes/src/lib/rewriteTemplates.test.ts

@@ -9,6 +9,7 @@ describe('rewriteTemplates', () => {
   })
 
   it('writes templates to the correct file paths', () => {
+    sinon.stub(fs, 'mkdirSync')
     const writeFileSyncStub = sinon.stub(fs, 'writeFileSync')
 
     const templates = {
@@ -35,6 +36,7 @@ describe('rewriteTemplates', () => {
   })
 
   it('throws an error when file cannot be written', () => {
+    sinon.stub(fs, 'mkdirSync')
     const writeFileSyncStub = sinon.stub(fs, 'writeFileSync')
 
     writeFileSyncStub.throws(new Error('Permission denied'))
@@ -58,7 +60,8 @@ describe('rewriteTemplates', () => {
     expect(writeFileSyncStub.callCount).to.equal(0)
   })
 
-  it('handles nested template paths correctly', () => {
+  it('creates intermediate directories for nested template paths', () => {
+    const mkdirSyncStub = sinon.stub(fs, 'mkdirSync')
     const writeFileSyncStub = sinon.stub(fs, 'writeFileSync')
 
     const templates = {
@@ -68,6 +71,15 @@ describe('rewriteTemplates', () => {
 
     rewriteTemplates('theme/path', templates)
 
+    expect(mkdirSyncStub.callCount).to.equal(2)
+    expect(mkdirSyncStub.firstCall.args).to.deep.equal([
+      'theme/path/templates/article_pages',
+      { recursive: true }
+    ])
+    expect(mkdirSyncStub.secondCall.args).to.deep.equal([
+      'theme/path/templates/custom_pages/deep/nested',
+      { recursive: true }
+    ])
     expect(writeFileSyncStub.callCount).to.equal(2)
     expect(writeFileSyncStub.firstCall.args).to.deep.equal([
       'theme/path/templates/article_pages/product_updates.hbs',

packages/zcli-themes/src/lib/rewriteTemplates.ts

@@ -1,5 +1,6 @@
 import { CLIError } from '@oclif/core/lib/errors'
 import * as fs from 'fs'
+import * as path from 'path'
 import * as chalk from 'chalk'
 
 export default function rewriteTemplates (themePath: string, templates: Record<string, string>) {
@@ -8,6 +9,7 @@ export default function rewriteTemplates (themePath: string, templates: Record<s
 
     if (typeof content === 'string') {
       try {
+        fs.mkdirSync(path.dirname(filePath), { recursive: true })
         fs.writeFileSync(filePath, content)
       } catch (error) {
         throw new CLIError(chalk.red(`Failed to write template file: ${filePath}`))

packages/zcli-themes/src/lib/validationErrorsToString.test.ts

@@ -39,4 +39,36 @@ describe('validationErrorsToString', () => {
     expect(string).to.contain('templates/new_request_page.hbs')
     expect(string).to.contain("'post_form' does not exist")
   })
+
+  it('includes the :line:column suffix when line or column is 0', () => {
+    const validationErrors = {
+      'templates/home_page.hbs': [
+        {
+          description: 'error at start of file',
+          line: 0,
+          column: 0,
+          length: 1
+        }
+      ]
+    }
+
+    const string = validationErrorsToString('theme/path', validationErrors)
+
+    expect(string).to.contain('theme/path/templates/home_page.hbs:0:0')
+  })
+
+  it('omits the suffix when line and column are not provided', () => {
+    const validationErrors = {
+      'templates/home_page.hbs': [
+        {
+          description: 'error without location'
+        }
+      ]
+    }
+
+    const string = validationErrorsToString('theme/path', validationErrors)
+
+    expect(string).to.contain('theme/path/templates/home_page.hbs\n')
+    expect(string).to.not.contain('home_page.hbs:')
+  })
 })

packages/zcli-themes/src/lib/validationErrorsToString.ts

@@ -6,7 +6,7 @@ export default function validationErrorsToString (themePath: string, validationE
 
   for (const [template, errors] of Object.entries(validationErrors)) {
     for (const { line, column, description } of errors) {
-      string += `\n${chalk.bold('Validation error')} ${themePath}/${template}${line && column ? `:${line}:${column}` : ''}\n ${description}\n`
+      string += `\n${chalk.bold('Validation error')} ${themePath}/${template}${line !== undefined && column !== undefined ? `:${line}:${column}` : ''}\n ${description}\n`
     }
   }