From cf1bc97f5662f6ffa8b55f83eb0cf963efc8cddb Mon Sep 17 00:00:00 2001 From: neverland Date: Tue, 11 Aug 2026 22:20:18 +0800 Subject: [PATCH 1/2] fix(fmt): propagate native gitignore errors --- packages/rstack/src/fmt/discoverPaths.ts | 100 ++++++++++++------ .../rstack/tests/fmt/discoverPaths.test.ts | 41 ++++++- 2 files changed, 107 insertions(+), 34 deletions(-) diff --git a/packages/rstack/src/fmt/discoverPaths.ts b/packages/rstack/src/fmt/discoverPaths.ts index 0850cec0..0a213400 100644 --- a/packages/rstack/src/fmt/discoverPaths.ts +++ b/packages/rstack/src/fmt/discoverPaths.ts @@ -188,13 +188,14 @@ class GitIgnoreFiles { } // Ignore files may disappear or become unreadable during traversal. - const loading = readFile(path.join(directoryPath, '.gitignore'), 'utf8') - .then((content) => { + const loading = readFile(path.join(directoryPath, '.gitignore'), 'utf8').then( + (content) => { const relativePath = toPosixPath(this.#resolveRelativePath(directoryPath)); this.#matcher ??= new (loadNativeBinding().GitIgnoreMatcher)(); this.#hasRules = this.#matcher.addSource(relativePath, content); - }) - .catch(() => undefined); + }, + () => undefined, + ); this.#loads.set(directoryPath, loading); return loading; @@ -204,11 +205,14 @@ class GitIgnoreFiles { const createTraversalOptions = ( gitIgnore: GitIgnoreFiles, ignoredDirNames: ReadonlySet, + signal: { aborted: boolean }, + onError: (error: unknown) => void, isIncluded?: (filePath: string) => boolean, isIgnored?: (filePath: string, isDirectory: boolean) => boolean, ) => { return { followSymlinks: false, + signal, ignore: (targetPath: string, targetContext: DirentLike) => { // With symlink following disabled, tiny-readdir always provides a Dirent here. const dirent = targetContext as Dirent; @@ -233,36 +237,40 @@ const createTraversalOptions = ( ); }, onDirents: async (dirents: Dirent[]) => { - const parentPath = getDirentParentPath(dirents[0]); - let hasGitIgnore = false; + try { + const parentPath = getDirentParentPath(dirents[0]); + let hasGitIgnore = false; - for (const dirent of dirents) { - if (dirent.name === '.gitignore') { - hasGitIgnore = true; + for (const dirent of dirents) { + if (dirent.name === '.gitignore') { + hasGitIgnore = true; + } } - } - if (hasGitIgnore) { - await gitIgnore.load(parentPath); - } - - const ignored = gitIgnore.matchDirents(parentPath, dirents); - if (typeof ignored === 'boolean') { - if (ignored) { - (dirents[0] as GitIgnoreDirent)[gitIgnored] = true; + if (hasGitIgnore) { + await gitIgnore.load(parentPath); } - } else if (typeof ignored === 'number') { - for (let index = 0; index < dirents.length; index++) { - if (ignored & (1 << index)) { - (dirents[index] as GitIgnoreDirent)[gitIgnored] = true; + + const ignored = gitIgnore.matchDirents(parentPath, dirents); + if (typeof ignored === 'boolean') { + if (ignored) { + (dirents[0] as GitIgnoreDirent)[gitIgnored] = true; } - } - } else if (ignored) { - for (let index = 0; index < ignored.length; index++) { - if (ignored[index] === 1) { - (dirents[index] as GitIgnoreDirent)[gitIgnored] = true; + } else if (typeof ignored === 'number') { + for (let index = 0; index < dirents.length; index++) { + if (ignored & (1 << index)) { + (dirents[index] as GitIgnoreDirent)[gitIgnored] = true; + } + } + } else if (ignored) { + for (let index = 0; index < ignored.length; index++) { + if (ignored[index] === 1) { + (dirents[index] as GitIgnoreDirent)[gitIgnored] = true; + } } } + } catch (error) { + onError(error); } return undefined; @@ -270,6 +278,37 @@ const createTraversalOptions = ( }; }; +const discoverDirectoryFiles = async ( + rootPath: string, + gitIgnore: GitIgnoreFiles, + ignoredDirNames: ReadonlySet, + isIncluded?: (filePath: string) => boolean, + isIgnored?: (filePath: string, isDirectory: boolean) => boolean, +): Promise => { + let failed = false; + let failure: unknown; + const signal = { aborted: false }; + const onError = (error: unknown): void => { + if (!failed) { + failed = true; + failure = error; + } + signal.aborted = true; + }; + + const result = await readdir( + rootPath, + createTraversalOptions(gitIgnore, ignoredDirNames, signal, onError, isIncluded, isIgnored), + ); + + // tiny-readdir only handles fulfilled onDirents promises, so rethrow after its counter settles. + if (failed) { + throw failure; + } + + return result.files; +}; + const normalizeGlob = (cwd: string, pattern: string): string => { const relativePattern = path.isAbsolute(pattern) ? path.relative(cwd, pattern) : pattern; return toPosixPath(relativePattern); @@ -426,12 +465,7 @@ const discoverFmtPaths = async ({ return globMatchers.some((matches) => matches(relativePath)); }; - return ( - await readdir( - rootPath, - createTraversalOptions(gitIgnore, ignoredDirNames, isIncluded, isIgnored), - ) - ).files; + return discoverDirectoryFiles(rootPath, gitIgnore, ignoredDirNames, isIncluded, isIgnored); }), ); diff --git a/packages/rstack/tests/fmt/discoverPaths.test.ts b/packages/rstack/tests/fmt/discoverPaths.test.ts index 1d4a4416..ac5d56cd 100644 --- a/packages/rstack/tests/fmt/discoverPaths.test.ts +++ b/packages/rstack/tests/fmt/discoverPaths.test.ts @@ -1,7 +1,8 @@ import { symlinkSync } from 'node:fs'; import path from 'node:path'; -import { expect, test } from 'rstack/test'; +import { expect, rs, test } from 'rstack/test'; import { discoverFmtPaths } from '../../src/fmt/discoverPaths.ts'; +import { loadNativeBinding } from '../../src/native/index.ts'; import { withTempProject, writeProjectFile } from './helpers.ts'; const relativePaths = (rootPath: string, files: string[]): string[] => @@ -165,6 +166,44 @@ test('keeps valid nested gitignore rules around normalized and malformed lines', }); }); +test.sequential('propagates errors while loading a nested gitignore', async () => { + await withTempProject(async (rootPath) => { + writeProjectFile(rootPath, 'src/.gitignore', '*.js\n'); + writeProjectFile(rootPath, 'src/index.js'); + const nativeError = new Error('Failed to add nested gitignore source'); + const addSource = rs + .spyOn(loadNativeBinding().GitIgnoreMatcher.prototype, 'addSource') + .mockImplementation(() => { + throw nativeError; + }); + + try { + await expect(discoverFmtPaths({ cwd: rootPath })).rejects.toBe(nativeError); + } finally { + addSource.mockRestore(); + } + }); +}); + +test.sequential('propagates errors from batched native gitignore matching', async () => { + await withTempProject(async (rootPath) => { + writeProjectFile(rootPath, '.gitignore', '*.js\n'); + writeProjectFile(rootPath, 'index.js'); + const nativeError = new Error('Failed to match gitignore entries'); + const matchBatch = rs + .spyOn(loadNativeBinding().GitIgnoreMatcher.prototype, 'isIgnoredBatchMask') + .mockImplementation(() => { + throw nativeError; + }); + + try { + await expect(discoverFmtPaths({ cwd: rootPath })).rejects.toBe(nativeError); + } finally { + matchBatch.mockRestore(); + } + }); +}); + test('lets explicit files bypass gitignore', async () => { await withTempProject(async (rootPath) => { writeProjectFile(rootPath, '.gitignore', '/generated/\n'); From 8583532641d019a2fd97965ec20a789daca978e7 Mon Sep 17 00:00:00 2001 From: neverland Date: Wed, 12 Aug 2026 08:51:37 +0800 Subject: [PATCH 2/2] test(fmt): avoid mocking native matcher prototype --- .../rstack/tests/fmt/discoverPaths.test.ts | 31 ++++--------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/packages/rstack/tests/fmt/discoverPaths.test.ts b/packages/rstack/tests/fmt/discoverPaths.test.ts index ac5d56cd..514cdc92 100644 --- a/packages/rstack/tests/fmt/discoverPaths.test.ts +++ b/packages/rstack/tests/fmt/discoverPaths.test.ts @@ -2,7 +2,7 @@ import { symlinkSync } from 'node:fs'; import path from 'node:path'; import { expect, rs, test } from 'rstack/test'; import { discoverFmtPaths } from '../../src/fmt/discoverPaths.ts'; -import { loadNativeBinding } from '../../src/native/index.ts'; +import * as nativeBinding from '../../src/native/index.ts'; import { withTempProject, writeProjectFile } from './helpers.ts'; const relativePaths = (rootPath: string, files: string[]): string[] => @@ -166,13 +166,13 @@ test('keeps valid nested gitignore rules around normalized and malformed lines', }); }); -test.sequential('propagates errors while loading a nested gitignore', async () => { +test('propagates native binding errors while loading a nested gitignore', async () => { await withTempProject(async (rootPath) => { writeProjectFile(rootPath, 'src/.gitignore', '*.js\n'); writeProjectFile(rootPath, 'src/index.js'); - const nativeError = new Error('Failed to add nested gitignore source'); - const addSource = rs - .spyOn(loadNativeBinding().GitIgnoreMatcher.prototype, 'addSource') + const nativeError = new Error('Failed to load native binding'); + const loadNativeBinding = rs + .spyOn(nativeBinding, 'loadNativeBinding') .mockImplementation(() => { throw nativeError; }); @@ -180,26 +180,7 @@ test.sequential('propagates errors while loading a nested gitignore', async () = try { await expect(discoverFmtPaths({ cwd: rootPath })).rejects.toBe(nativeError); } finally { - addSource.mockRestore(); - } - }); -}); - -test.sequential('propagates errors from batched native gitignore matching', async () => { - await withTempProject(async (rootPath) => { - writeProjectFile(rootPath, '.gitignore', '*.js\n'); - writeProjectFile(rootPath, 'index.js'); - const nativeError = new Error('Failed to match gitignore entries'); - const matchBatch = rs - .spyOn(loadNativeBinding().GitIgnoreMatcher.prototype, 'isIgnoredBatchMask') - .mockImplementation(() => { - throw nativeError; - }); - - try { - await expect(discoverFmtPaths({ cwd: rootPath })).rejects.toBe(nativeError); - } finally { - matchBatch.mockRestore(); + loadNativeBinding.mockRestore(); } }); });