Merge pull request #1748 from maizzle/fix/general-bugs

fix: general framework bugs

Author: cossssmin

src/build.ts

@@ -1,5 +1,5 @@
 import { mkdirSync, cpSync, existsSync, rmSync } from 'node:fs'
-import { resolve, dirname, relative, join } from 'node:path'
+import { resolve, dirname, relative, join, parse as parsePath, sep } from 'node:path'
 import { fileURLToPath } from 'node:url'
 import { availableParallelism } from 'node:os'
 import { glob } from 'tinyglobby'
@@ -30,80 +30,93 @@ export async function build(configInput?: Partial<MaizzleConfig> | string): Prom
   const start = Date.now()
   const spinner = ora({ text: 'Building templates...', spinner: 'circleHalves' }).start()
 
-  const config = await resolveConfig(configInput)
-
-  const events = new EventManager()
-  events.registerConfig(config)
-  await events.fireBeforeCreate({ config })
+  try {
+    const config = await resolveConfig(configInput)
 
-  const outputPath = resolve(config.output?.path ?? 'dist')
-  const outputExtension = config.output?.extension ?? 'html'
+    const events = new EventManager()
+    events.registerConfig(config)
+    await events.fireBeforeCreate({ config })
 
-  const contentPatterns = config.content ?? ['emails/**/*.vue']
-  const contentBase = computeContentBase(contentPatterns)
-  const templateFiles = await glob(contentPatterns)
+    const outputPath = resolve(config.output?.path ?? 'dist')
+    const outputExtension = config.output?.extension ?? 'html'
 
-  if (templateFiles.length === 0) {
-    spinner.succeed('No templates found')
-    return { files: [], config }
-  }
+    const contentPatterns = config.content ?? ['emails/**/*.vue']
+    const contentBase = computeContentBase(contentPatterns)
+    const templateFiles = await glob(contentPatterns)
 
-  // Clear the output directory before writing fresh output
-  if (existsSync(outputPath)) {
-    rmSync(outputPath, { recursive: true, force: true })
-  }
+    if (templateFiles.length === 0) {
+      spinner.succeed('No templates found')
+      return { files: [], config }
+    }
 
-  const outputFiles: string[] = []
-  let droppedAfterBuild = 0
+    // Clear the output directory before writing fresh output. Guard against a
+    // misconfigured output.path (e.g. '.', '', '../..') that resolves to the
+    // project root or a parent of it — rmSync would wipe the whole project.
+    const cwd = process.cwd()
+    if (outputPath === parsePath(outputPath).root || outputPath === cwd || cwd.startsWith(outputPath + sep)) {
+      throw new Error(`Refusing to clear output path "${outputPath}": it is the filesystem root, the current working directory, or a parent of it. Set output.path to a subdirectory like "dist".`)
+    }
 
-  const parallel = resolveParallel(config, templateFiles.length, configInput)
+    if (existsSync(outputPath)) {
+      rmSync(outputPath, { recursive: true, force: true })
+    }
 
-  if (parallel.enabled) {
-    spinner.text = `Building ${templateFiles.length} templates across ${parallel.workers} workers...`
+    const outputFiles: string[] = []
+    let droppedAfterBuild = 0
 
-    const result = await runParallelBuild({
-      templateFiles,
-      workers: parallel.workers,
-      config,
-      configInput,
-      outputPath,
-      outputExtension,
-      contentBase,
-    })
+    const parallel = resolveParallel(config, templateFiles.length, configInput)
 
-    outputFiles.push(...result.files)
-    droppedAfterBuild = result.sfcAfterBuildCount
+    if (parallel.enabled) {
+      spinner.text = `Building ${templateFiles.length} templates across ${parallel.workers} workers...`
 
-    await copyStatic(config, outputPath)
-    await events.fireAfterBuild({ files: outputFiles, config })
-  } else {
-    const renderer = await createRenderer({ markdown: config.markdown, root: config.root, componentDirs: normalizeComponentSources(config.components?.source, process.cwd()), vite: config.vite })
+      const result = await runParallelBuild({
+        templateFiles,
+        workers: parallel.workers,
+        config,
+        configInput,
+        outputPath,
+        outputExtension,
+        contentBase,
+      })
 
-    try {
-      for (const templatePath of templateFiles) {
-        const { files } = await buildTemplate(templatePath, { config, renderer, events, outputPath, outputExtension, contentBase })
-        outputFiles.push(...files)
-      }
+      outputFiles.push(...result.files)
+      droppedAfterBuild = result.sfcAfterBuildCount
 
       await copyStatic(config, outputPath)
       await events.fireAfterBuild({ files: outputFiles, config })
-    } finally {
-      await renderer.close()
+    } else {
+      const renderer = await createRenderer({ markdown: config.markdown, root: config.root, componentDirs: normalizeComponentSources(config.components?.source, process.cwd()), vite: config.vite })
+
+      try {
+        for (const templatePath of templateFiles) {
+          const { files } = await buildTemplate(templatePath, { config, renderer, events, outputPath, outputExtension, contentBase })
+          outputFiles.push(...files)
+        }
+
+        await copyStatic(config, outputPath)
+        await events.fireAfterBuild({ files: outputFiles, config })
+      } finally {
+        await renderer.close()
+      }
     }
-  }
 
-  if (droppedAfterBuild > 0) {
-    console.warn(`[maizzle] Skipped ${droppedAfterBuild} SFC-registered afterBuild handler(s): afterBuild can't run inside a parallel build worker. Move build-completion logic to the config's afterBuild hook.`)
-  }
+    if (droppedAfterBuild > 0) {
+      console.warn(`[maizzle] Skipped ${droppedAfterBuild} SFC-registered afterBuild handler(s): afterBuild can't run inside a parallel build worker. Move build-completion logic to the config's afterBuild hook.`)
+    }
 
-  const duration = ((Date.now() - start) / 1000).toFixed(2)
-  const count = outputFiles.length
-  spinner.stopAndPersist({
-    symbol: '✅',
-    text: `Built ${count} template${count !== 1 ? 's' : ''} in ${duration}s`,
-  })
+    const duration = ((Date.now() - start) / 1000).toFixed(2)
+    const count = outputFiles.length
+    spinner.stopAndPersist({
+      symbol: '✅',
+      text: `Built ${count} template${count !== 1 ? 's' : ''} in ${duration}s`,
+    })
 
-  return { files: outputFiles, config }
+    return { files: outputFiles, config }
+  } catch (err) {
+    // Stop the spinner so a thrown error doesn't leave it spinning forever.
+    if (spinner.isSpinning) spinner.fail('Build failed')
+    throw err
+  }
 }
 
 /**
@@ -225,10 +238,21 @@ async function copyStatic(config: MaizzleConfig, outputPath: string): Promise<vo
   const sources = config.static?.source ?? ['public/**/*.*']
   const destination = config.static?.destination ?? 'public'
 
+  // One glob call so negation patterns still apply across the whole set.
   const files = await glob(sources)
 
+  // Absolute base dir to strip, per positive source pattern. Each file keeps
+  // the structure under its own pattern's base — using only sources[0] sends
+  // files from other roots to the wrong place (or escaping the output dir).
+  const bases = sources.filter(s => !s.startsWith('!')).map(staticBase)
+
   for (const file of files) {
-    const destPath = join(outputPath, destination, relative(dirname(sources[0]).replace(/\*.*$/, ''), file))
+    const abs = resolve(file)
+    const base = bases
+      .filter(b => abs === b || abs.startsWith(b + sep))
+      .sort((a, b) => b.length - a.length)[0] ?? bases[0] ?? resolve('.')
+
+    const destPath = join(outputPath, destination, relative(base, abs))
     const destDir = dirname(destPath)
 
     if (!existsSync(destDir)) {
@@ -238,3 +262,10 @@ async function copyStatic(config: MaizzleConfig, outputPath: string): Promise<vo
     cpSync(file, destPath)
   }
 }
+
+/** Absolute static (non-glob) prefix of a source pattern, used as the strip base. */
+function staticBase(pattern: string): string {
+  const staticPart = pattern.split(/[*{?[]/)[0]
+  // Treat both separators as trailing: resolved patterns use '\' on Windows.
+  return resolve(/[/\\]$/.test(staticPart) ? staticPart : dirname(staticPart))
+}

src/serve.ts

@@ -199,23 +199,37 @@ function maizzleDevPlugin(
         server.watcher.add(watchPath)
       }
 
-      server.watcher.on('add', async (file) => {
+      /**
+       * Serialize watcher work onto one chain. The change handler closes and
+       * recreates the renderer across awaits; without serialization a second
+       * event firing mid-reload closes a stale renderer and leaks the new one.
+       * Errors are caught so one failed task doesn't break the chain.
+       */
+      let watcherChain: Promise<void> = Promise.resolve()
+      const enqueue = (task: () => Promise<void>): Promise<void> => {
+        watcherChain = watcherChain.then(task).catch((err) => {
+          console.error('[maizzle] watcher task failed:', err)
+        })
+        return watcherChain
+      }
+
+      server.watcher.on('add', file => enqueue(async () => {
         if (isTemplateFile(file)) {
           await renderer.invalidateAll()
           bumpGeneration()
           server.ws.send({ type: 'custom', event: 'maizzle:templates-changed' })
         }
-      })
+      }))
 
-      server.watcher.on('unlink', async (file) => {
+      server.watcher.on('unlink', file => enqueue(async () => {
         if (isTemplateFile(file)) {
           await renderer.invalidateAll()
           bumpGeneration()
           server.ws.send({ type: 'custom', event: 'maizzle:templates-changed' })
         }
-      })
+      }))
 
-      server.watcher.on('change', async (file) => {
+      server.watcher.on('change', file => enqueue(async () => {
         if (isWatchedFile(file)) {
           config = await resolveConfig(configInput)
 
@@ -249,7 +263,7 @@ function maizzleDevPlugin(
         ) {
           server.ws.send({ type: 'custom', event: 'maizzle:template-updated', data: { file } })
         }
-      })
+      }))
 
       // API middleware (before Vite's middleware)
       server.middlewares.use(async (req: any, res: any, next: any) => {

src/tests/build.test.ts

@@ -5,6 +5,22 @@ import { tmpdir, availableParallelism } from 'node:os'
 import { build, resolveParallel } from '../build.ts'
 import { computeContentBase } from '../render/buildTemplate.ts'
 
+// Track ora.fail() calls so we can assert the spinner is stopped on error.
+const oraState = vi.hoisted(() => ({ failCalls: 0 }))
+vi.mock('ora', () => ({
+  default: () => {
+    const s: any = {
+      isSpinning: false,
+      text: '',
+      start() { s.isSpinning = true; return s },
+      stopAndPersist() { s.isSpinning = false; return s },
+      succeed() { s.isSpinning = false; return s },
+      fail() { s.isSpinning = false; oraState.failCalls++; return s },
+    }
+    return s
+  },
+}))
+
 function createTempProject() {
   const dir = mkdtempSync(join(tmpdir(), 'maizzle-build-'))
   return dir
@@ -64,6 +80,34 @@ describe('build', () => {
     expect(result.files[0]).toContain('/dist/')
   })
 
+  it('stops the spinner when the build throws', async () => {
+    writeSfc(tempDir, 'emails/test.vue', `
+      <template><div>Test</div></template>
+    `)
+
+    oraState.failCalls = 0
+
+    await expect(build({ beforeCreate() { throw new Error('boom') } })).rejects.toThrow('boom')
+
+    // Without the try/catch the spinner would still be spinning (fail uncalled).
+    expect(oraState.failCalls).toBe(1)
+  })
+
+  it('refuses to clear an output path that is the cwd', async () => {
+    writeSfc(tempDir, 'emails/test.vue', `
+      <template>
+        <p>Test</p>
+      </template>
+    `)
+
+    // output.path '.' resolves to the project root (cwd) — clearing it would
+    // wipe the whole project, so the build must reject instead.
+    await expect(build({ output: { path: '.' } })).rejects.toThrow(/Refusing to clear output path/)
+
+    // The source template must still be intact (not deleted).
+    expect(existsSync(join(tempDir, 'emails/test.vue'))).toBe(true)
+  })
+
   it('respects output.extension config', async () => {
     writeSfc(tempDir, 'emails/test.vue', `
       <template>
@@ -762,6 +806,31 @@ describe('build', () => {
     expect(readFileSync(join(outputDir, 'public/images/logo.png'), 'utf-8')).toBe('fake-png-data')
   })
 
+  it('copies static assets from multiple source roots using each pattern base', async () => {
+    writeSfc(tempDir, 'emails/test.vue', `
+      <template><div>Test</div></template>
+    `)
+
+    mkdirSync(join(tempDir, 'public/img'), { recursive: true })
+    writeFileSync(join(tempDir, 'public/img/logo.png'), 'logo')
+    mkdirSync(join(tempDir, 'src/assets'), { recursive: true })
+    writeFileSync(join(tempDir, 'src/assets/app.css'), 'css')
+
+    await build({
+      static: {
+        source: [join(tempDir, 'public/**/*.*'), join(tempDir, 'src/assets/**/*.*')],
+        destination: 'assets',
+      },
+    })
+
+    const out = join(tempDir, 'dist/assets')
+    // Each file keeps the structure under its OWN pattern's base.
+    expect(existsSync(join(out, 'img/logo.png'))).toBe(true)
+    expect(existsSync(join(out, 'app.css'))).toBe(true)
+    // The second-root file must not escape the destination via ../ traversal.
+    expect(existsSync(join(tempDir, 'dist/src/assets/app.css'))).toBe(false)
+  })
+
   it('clears existing output directory before building', async () => {
     const outputDir = join(tempDir, 'dist')
     mkdirSync(outputDir, { recursive: true })

src/tests/serve.test.ts

@@ -5,6 +5,10 @@ import { tmpdir } from 'node:os'
 import type { ViteDevServer } from 'vite'
 import { serve } from '../serve.ts'
 import { getActiveRenderer } from '../render/active.ts'
+import * as rendererMod from '../render/createRenderer.ts'
+import type { Renderer } from '../render/createRenderer.ts'
+
+const realCreateRenderer = rendererMod.createRenderer
 
 describe('serve dev server', () => {
   let tempDir: string
@@ -57,6 +61,49 @@ describe('serve dev server', () => {
     }, { timeout: 15000, interval: 100 })
   }, 30000)
 
+  it('serializes rapid config changes so no renderer is leaked', async () => {
+    writeFileSync(join(tempDir, 'maizzle.config.js'), 'export default {}\n')
+
+    // Wrap createRenderer to record every instance and whether it was closed.
+    // The race closes the same (stale) renderer repeatedly and leaks the
+    // intermediate ones; serialized reloads close each before the next.
+    const created: Renderer[] = []
+    const closed = new Set<Renderer>()
+    const spy = vi.spyOn(rendererMod, 'createRenderer').mockImplementation(async (opts) => {
+      const r = await realCreateRenderer(opts)
+      created.push(r)
+      const origClose = r.close.bind(r)
+      r.close = async () => { closed.add(r); return origClose() }
+      return r
+    })
+
+    try {
+      server = await serve({ port: 3157, silent: true })
+      const before = getActiveRenderer()
+      expect(before).toBeTruthy()
+
+      const cfg = resolve(tempDir, 'maizzle.config.js')
+      server.watcher.emit('change', cfg)
+      server.watcher.emit('change', cfg)
+
+      await vi.waitFor(() => {
+        expect(getActiveRenderer()).not.toBe(before)
+      }, { timeout: 20000, interval: 100 })
+
+      // Let in-flight reloads settle (close() carries a 600ms dts drain).
+      await new Promise(r => setTimeout(r, 2000))
+
+      // Every renderer ever created except the current active one must have
+      // been closed. A leaked intermediate renderer fails this.
+      const active = getActiveRenderer()
+      for (const r of created) {
+        if (r !== active) expect(closed.has(r)).toBe(true)
+      }
+    } finally {
+      spy.mockRestore()
+    }
+  }, 40000)
+
   it('clears the active renderer on close', async () => {
     server = await serve({ port: 3157, silent: true })
     expect(getActiveRenderer()).toBeTruthy()