From 60f4dd5e8c347850272a20b85b9c1014c4cef886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Re=C3=A9?= Date: Tue, 30 Dec 2025 11:22:00 +0100 Subject: [PATCH 1/4] Add test demonstrating srcDir glob pattern issue Test shows that parentheses syntax (src|__tests__) silently fails with glob but works with chokidar (watch mode). The glob package treats it as a literal string while chokidar interprets it as an extglob pattern. --- packages/cli/src/glob.rescript.test.ts | 141 +++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 packages/cli/src/glob.rescript.test.ts diff --git a/packages/cli/src/glob.rescript.test.ts b/packages/cli/src/glob.rescript.test.ts new file mode 100644 index 00000000..c4d6db83 --- /dev/null +++ b/packages/cli/src/glob.rescript.test.ts @@ -0,0 +1,141 @@ +import { globSync } from 'glob'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +describe('srcDir glob pattern matching', () => { + let testDir: string; + let srcDir: string; + let testsDir: string; + + beforeEach(() => { + // Create a temporary directory structure + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pgtyped-test-')); + srcDir = path.join(testDir, 'src'); + testsDir = path.join(testDir, '__tests__'); + + // Create directories + fs.mkdirSync(srcDir, { recursive: true }); + fs.mkdirSync(testsDir, { recursive: true }); + fs.mkdirSync(path.join(srcDir, 'queries'), { recursive: true }); + fs.mkdirSync(path.join(testsDir, 'queries'), { recursive: true }); + + // Create test SQL files + fs.writeFileSync( + path.join(srcDir, 'queries', 'users.sql'), + '/* @name GetUsers */\nSELECT * FROM users;', + ); + fs.writeFileSync( + path.join(testsDir, 'queries', 'test.sql'), + '/* @name GetTestData */\nSELECT * FROM test_data;', + ); + }); + + afterEach(() => { + // Cleanup + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + test('working srcDir pattern finds SQL files', () => { + // This is the correct way to specify srcDir + const srcDirPattern = path.join(testDir, 'src'); + const include = '**/*.sql'; + const pattern = `${srcDirPattern}/**/${include}`; + + const files = globSync(pattern); + expect(files.length).toBe(1); + expect(files[0]).toContain('users.sql'); + }); + + test('problematic srcDir with parentheses syntax finds NO files', () => { + // This is the pattern reported by the user - using parentheses for alternation + // This does NOT work because (src|__tests__) is not valid glob syntax + const srcDirPattern = `${testDir}/(src|__tests__)`; + const include = '**/*.sql'; + const pattern = `${srcDirPattern}/**/${include}`; + + const files = globSync(pattern); + // This will find 0 files because (src|__tests__) is not valid glob + expect(files.length).toBe(0); + }); + + test('correct brace syntax for multiple directories finds files', () => { + // This is the correct glob syntax for matching multiple directories + const srcDirPattern = `${testDir}/{src,__tests__}`; + const include = '**/*.sql'; + const pattern = `${srcDirPattern}/**/${include}`; + + const files = globSync(pattern); + // Should find both files + expect(files.length).toBe(2); + }); + + test('srcDir with embedded wildcards causes pattern issues', () => { + // The user's config had srcDir: "./(src|__tests__)/**/*" + // Even with correct brace syntax, having **/* in srcDir is problematic + const srcDirWithWildcard = `${testDir}/{src,__tests__}/**/*`; + const include = '**/*.sql'; + const pattern = `${srcDirWithWildcard}/**/${include}`; + + // Pattern becomes: {src,__tests__}/**/*/**/**/*.sql + // This works but is redundant and confusing + const files = globSync(pattern); + // May still work, but the pattern is unnecessarily complex + expect(files.length).toBeGreaterThanOrEqual(0); + }); + + test('srcDir should NOT contain ** wildcards', () => { + // Best practice: srcDir should be a simple directory path, not a glob pattern + // The transform.include already handles the file matching + + // CORRECT: + const correctSrcDir = `${testDir}/src`; + const correctPattern = `${correctSrcDir}/**/${'**/*.sql'}`; + const correctFiles = globSync(correctPattern); + expect(correctFiles.length).toBe(1); + + // INCORRECT (but still works with proper brace syntax): + const redundantSrcDir = `${testDir}/src/**/*`; + const redundantPattern = `${redundantSrcDir}/**/${'**/*.sql'}`; + const redundantFiles = globSync(redundantPattern); + // This still works but is overly complex + expect(redundantFiles.length).toBeGreaterThanOrEqual(0); + }); + + describe('demonstrates the silent failure issue', () => { + test('invalid srcDir pattern silently returns empty array', () => { + // This is the exact issue: with an invalid srcDir pattern, + // glob returns an empty array without any error + const invalidSrcDir = `${testDir}/(src|__tests__)/**/*`; + const include = '**/*.sql'; + const pattern = `${invalidSrcDir}/**/${include}`; + + // glob silently returns empty array - no error thrown + expect(() => { + const files = globSync(pattern); + expect(files).toEqual([]); + }).not.toThrow(); + + // The CLI would then process 0 files and exit successfully + // without any warning that the pattern didn't match anything + }); + + test('CLI should warn when no files are matched', () => { + // This test documents the expected behavior: + // When srcDir pattern matches 0 files, the CLI should warn the user + const emptyDir = path.join(testDir, 'empty'); + fs.mkdirSync(emptyDir, { recursive: true }); + + const srcDirPattern = emptyDir; + const include = '**/*.sql'; + const pattern = `${srcDirPattern}/**/${include}`; + + const files = globSync(pattern); + expect(files.length).toBe(0); + + // TODO: The CLI currently doesn't warn about this. + // It should output something like: + // "Warning: No files matched pattern: /path/to/empty/**/**/*.sql" + }); + }); +}); From 8a4a723c12a2c3503fd6ff4c1feb625649cfe714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Re=C3=A9?= Date: Tue, 30 Dec 2025 11:34:20 +0100 Subject: [PATCH 2/4] Use chokidar for file matching in build mode Replace globSync with chokidar-based file matching to ensure consistent pattern handling between watch and build modes. Chokidar (via picomatch) supports extglob patterns like (src|__tests__) while the glob package treats them as literal strings, causing silent failures when no files match. --- packages/cli/src/index.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 9ed21362..ad0c4a8f 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -3,7 +3,6 @@ import { startup } from 'pgtyped-rescript-query'; import { AsyncQueue } from '@pgtyped/wire'; import chokidar from 'chokidar'; -import { globSync } from 'glob'; import nun from 'nunjucks'; import yargs from 'yargs'; import { hideBin } from 'yargs/helpers'; @@ -17,6 +16,19 @@ import WorkerPool from 'piscina'; nun.configure({ autoescape: false }); +/** Uses chokidar to collect files matching a glob pattern */ +function getMatchedFiles(pattern: string): Promise { + return new Promise((resolve) => { + const files: string[] = []; + const watcher = chokidar.watch(pattern, { persistent: false }); + watcher.on('add', (filePath) => files.push(filePath)); + watcher.on('ready', () => { + watcher.close(); + resolve(files); + }); + }); +} + interface TransformJob { files: string[]; transform: TransformConfig; @@ -113,10 +125,10 @@ async function main( .on('change', cb); } else { /** - * If the user didn't provide the -f paramter, we're using the list of files we got from glob. - * If he did, we're using glob file list to detect if his provided file should be used with this transform. + * If the user didn't provide the -f parameter, we're using the list of files we got from chokidar. + * If he did, we're using the file list to detect if his provided file should be used with this transform. */ - let fileList = globSync(pattern); + let fileList = await getMatchedFiles(pattern); if (fileOverride) { fileList = fileList.includes(fileOverride) ? [fileOverride] : []; if (fileList.length > 0) { From b3c259ebf4ba2c3599622ba61bdb0a38e599ca2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Re=C3=A9?= Date: Tue, 30 Dec 2025 11:40:04 +0100 Subject: [PATCH 3/4] Update tests to reflect chokidar-based file matching Tests now verify that: - Parentheses extglob syntax (src|__tests__) works with chokidar - Both brace {a,b} and extglob (a|b) patterns are supported - Documents the glob vs chokidar behavior difference --- packages/cli/src/glob.rescript.test.ts | 121 ++++++++++++------------- 1 file changed, 56 insertions(+), 65 deletions(-) diff --git a/packages/cli/src/glob.rescript.test.ts b/packages/cli/src/glob.rescript.test.ts index c4d6db83..e6f18faf 100644 --- a/packages/cli/src/glob.rescript.test.ts +++ b/packages/cli/src/glob.rescript.test.ts @@ -1,8 +1,22 @@ +import chokidar from 'chokidar'; import { globSync } from 'glob'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; +/** Uses chokidar to collect files matching a glob pattern (same as CLI) */ +function getMatchedFiles(pattern: string): Promise { + return new Promise((resolve) => { + const files: string[] = []; + const watcher = chokidar.watch(pattern, { persistent: false }); + watcher.on('add', (filePath) => files.push(filePath)); + watcher.on('ready', () => { + watcher.close(); + resolve(files); + }); + }); +} + describe('srcDir glob pattern matching', () => { let testDir: string; let srcDir: string; @@ -36,106 +50,83 @@ describe('srcDir glob pattern matching', () => { fs.rmSync(testDir, { recursive: true, force: true }); }); - test('working srcDir pattern finds SQL files', () => { + test('working srcDir pattern finds SQL files', async () => { // This is the correct way to specify srcDir const srcDirPattern = path.join(testDir, 'src'); const include = '**/*.sql'; const pattern = `${srcDirPattern}/**/${include}`; - const files = globSync(pattern); + const files = await getMatchedFiles(pattern); expect(files.length).toBe(1); expect(files[0]).toContain('users.sql'); }); - test('problematic srcDir with parentheses syntax finds NO files', () => { - // This is the pattern reported by the user - using parentheses for alternation - // This does NOT work because (src|__tests__) is not valid glob syntax + test('parentheses syntax works with chokidar (extglob)', async () => { + // Chokidar (via picomatch) supports extglob patterns like (src|__tests__) + // This pattern means "src OR __tests__" const srcDirPattern = `${testDir}/(src|__tests__)`; const include = '**/*.sql'; const pattern = `${srcDirPattern}/**/${include}`; - const files = globSync(pattern); - // This will find 0 files because (src|__tests__) is not valid glob - expect(files.length).toBe(0); + const files = await getMatchedFiles(pattern); + // Chokidar finds files in both directories + expect(files.length).toBe(2); }); - test('correct brace syntax for multiple directories finds files', () => { - // This is the correct glob syntax for matching multiple directories + test('brace syntax also works for multiple directories', async () => { + // Both brace syntax {a,b} and extglob (a|b) work with chokidar const srcDirPattern = `${testDir}/{src,__tests__}`; const include = '**/*.sql'; const pattern = `${srcDirPattern}/**/${include}`; - const files = globSync(pattern); - // Should find both files + const files = await getMatchedFiles(pattern); expect(files.length).toBe(2); }); - test('srcDir with embedded wildcards causes pattern issues', () => { + test('srcDir with embedded wildcards still works', async () => { // The user's config had srcDir: "./(src|__tests__)/**/*" - // Even with correct brace syntax, having **/* in srcDir is problematic - const srcDirWithWildcard = `${testDir}/{src,__tests__}/**/*`; + // Even with **/* in srcDir, chokidar handles it + const srcDirWithWildcard = `${testDir}/(src|__tests__)/**/*`; const include = '**/*.sql'; const pattern = `${srcDirWithWildcard}/**/${include}`; - // Pattern becomes: {src,__tests__}/**/*/**/**/*.sql - // This works but is redundant and confusing - const files = globSync(pattern); - // May still work, but the pattern is unnecessarily complex - expect(files.length).toBeGreaterThanOrEqual(0); - }); - - test('srcDir should NOT contain ** wildcards', () => { - // Best practice: srcDir should be a simple directory path, not a glob pattern - // The transform.include already handles the file matching - - // CORRECT: - const correctSrcDir = `${testDir}/src`; - const correctPattern = `${correctSrcDir}/**/${'**/*.sql'}`; - const correctFiles = globSync(correctPattern); - expect(correctFiles.length).toBe(1); - - // INCORRECT (but still works with proper brace syntax): - const redundantSrcDir = `${testDir}/src/**/*`; - const redundantPattern = `${redundantSrcDir}/**/${'**/*.sql'}`; - const redundantFiles = globSync(redundantPattern); - // This still works but is overly complex - expect(redundantFiles.length).toBeGreaterThanOrEqual(0); + // Pattern becomes complex but chokidar handles it + const files = await getMatchedFiles(pattern); + expect(files.length).toBe(2); }); - describe('demonstrates the silent failure issue', () => { - test('invalid srcDir pattern silently returns empty array', () => { - // This is the exact issue: with an invalid srcDir pattern, - // glob returns an empty array without any error - const invalidSrcDir = `${testDir}/(src|__tests__)/**/*`; + describe('glob vs chokidar behavior difference', () => { + test('glob does NOT support parentheses extglob syntax', () => { + // This documents why we switched from glob to chokidar + const srcDirPattern = `${testDir}/(src|__tests__)`; const include = '**/*.sql'; - const pattern = `${invalidSrcDir}/**/${include}`; - - // glob silently returns empty array - no error thrown - expect(() => { - const files = globSync(pattern); - expect(files).toEqual([]); - }).not.toThrow(); + const pattern = `${srcDirPattern}/**/${include}`; - // The CLI would then process 0 files and exit successfully - // without any warning that the pattern didn't match anything + // glob treats (src|__tests__) as a literal directory name + const files = globSync(pattern); + expect(files.length).toBe(0); }); - test('CLI should warn when no files are matched', () => { - // This test documents the expected behavior: - // When srcDir pattern matches 0 files, the CLI should warn the user - const emptyDir = path.join(testDir, 'empty'); - fs.mkdirSync(emptyDir, { recursive: true }); - - const srcDirPattern = emptyDir; + test('chokidar DOES support parentheses extglob syntax', async () => { + const srcDirPattern = `${testDir}/(src|__tests__)`; const include = '**/*.sql'; const pattern = `${srcDirPattern}/**/${include}`; - const files = globSync(pattern); - expect(files.length).toBe(0); - - // TODO: The CLI currently doesn't warn about this. - // It should output something like: - // "Warning: No files matched pattern: /path/to/empty/**/**/*.sql" + // chokidar interprets (src|__tests__) as "src OR __tests__" + const files = await getMatchedFiles(pattern); + expect(files.length).toBe(2); }); }); + + test('empty directory returns empty array without error', async () => { + const emptyDir = path.join(testDir, 'empty'); + fs.mkdirSync(emptyDir, { recursive: true }); + + const srcDirPattern = emptyDir; + const include = '**/*.sql'; + const pattern = `${srcDirPattern}/**/${include}`; + + const files = await getMatchedFiles(pattern); + expect(files.length).toBe(0); + }); }); From b2812f4001066663c5cc7114e3f5a33488ac4438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Re=C3=A9?= Date: Tue, 30 Dec 2025 11:43:18 +0100 Subject: [PATCH 4/4] Remove glob usage from tests, use only chokidar-based getMatchedFiles --- packages/cli/src/glob.rescript.test.ts | 39 +++----------------------- 1 file changed, 4 insertions(+), 35 deletions(-) diff --git a/packages/cli/src/glob.rescript.test.ts b/packages/cli/src/glob.rescript.test.ts index e6f18faf..f2043817 100644 --- a/packages/cli/src/glob.rescript.test.ts +++ b/packages/cli/src/glob.rescript.test.ts @@ -1,5 +1,4 @@ import chokidar from 'chokidar'; -import { globSync } from 'glob'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -50,8 +49,7 @@ describe('srcDir glob pattern matching', () => { fs.rmSync(testDir, { recursive: true, force: true }); }); - test('working srcDir pattern finds SQL files', async () => { - // This is the correct way to specify srcDir + test('simple srcDir pattern finds SQL files', async () => { const srcDirPattern = path.join(testDir, 'src'); const include = '**/*.sql'; const pattern = `${srcDirPattern}/**/${include}`; @@ -61,20 +59,16 @@ describe('srcDir glob pattern matching', () => { expect(files[0]).toContain('users.sql'); }); - test('parentheses syntax works with chokidar (extglob)', async () => { - // Chokidar (via picomatch) supports extglob patterns like (src|__tests__) - // This pattern means "src OR __tests__" + test('parentheses extglob syntax (src|__tests__) finds files in both directories', async () => { const srcDirPattern = `${testDir}/(src|__tests__)`; const include = '**/*.sql'; const pattern = `${srcDirPattern}/**/${include}`; const files = await getMatchedFiles(pattern); - // Chokidar finds files in both directories expect(files.length).toBe(2); }); - test('brace syntax also works for multiple directories', async () => { - // Both brace syntax {a,b} and extglob (a|b) work with chokidar + test('brace syntax {src,__tests__} finds files in both directories', async () => { const srcDirPattern = `${testDir}/{src,__tests__}`; const include = '**/*.sql'; const pattern = `${srcDirPattern}/**/${include}`; @@ -83,41 +77,16 @@ describe('srcDir glob pattern matching', () => { expect(files.length).toBe(2); }); - test('srcDir with embedded wildcards still works', async () => { + test('srcDir with embedded wildcards (src|__tests__)/**/* works', async () => { // The user's config had srcDir: "./(src|__tests__)/**/*" - // Even with **/* in srcDir, chokidar handles it const srcDirWithWildcard = `${testDir}/(src|__tests__)/**/*`; const include = '**/*.sql'; const pattern = `${srcDirWithWildcard}/**/${include}`; - // Pattern becomes complex but chokidar handles it const files = await getMatchedFiles(pattern); expect(files.length).toBe(2); }); - describe('glob vs chokidar behavior difference', () => { - test('glob does NOT support parentheses extglob syntax', () => { - // This documents why we switched from glob to chokidar - const srcDirPattern = `${testDir}/(src|__tests__)`; - const include = '**/*.sql'; - const pattern = `${srcDirPattern}/**/${include}`; - - // glob treats (src|__tests__) as a literal directory name - const files = globSync(pattern); - expect(files.length).toBe(0); - }); - - test('chokidar DOES support parentheses extglob syntax', async () => { - const srcDirPattern = `${testDir}/(src|__tests__)`; - const include = '**/*.sql'; - const pattern = `${srcDirPattern}/**/${include}`; - - // chokidar interprets (src|__tests__) as "src OR __tests__" - const files = await getMatchedFiles(pattern); - expect(files.length).toBe(2); - }); - }); - test('empty directory returns empty array without error', async () => { const emptyDir = path.join(testDir, 'empty'); fs.mkdirSync(emptyDir, { recursive: true });