Repository navigation
Conversation
… the edit that turned on errors dropped
…t one on its own, skipped by default and reported with errors
…where the walker reads the disk through the output path
…th the "\" separator
…X name keeps its backslash, with the default separator too
…nverting them only to a custom separator
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With
.withPathSeparator("\\")on Linux or macOS, a crawl reads nothing: the walker hands the file system the output path, soreaddirgetsnode_modules\and fails withENOENT. On Windows a/separator happens to work with the file system, which is why the bug only shows on POSIX. "crawl all files and invert path separator" only passed before because the error was suppressed and an empty result satisfiesevery().The fix: the walker gives the file system native paths and the custom separator only to what it outputs.
readdir,realpath,stat,state.root,state.visitedandstate.symlinksall usepath.sep.normalizePathnow normalizes in the platform's own syntax and ends the path withpath.sep. On POSIX a backslash is part of a name, asfsreads it, so a root or a symlink target such asliteral\is read asliteral\, not asliteral/. On Windows nothing changes: a/still gets the path cleaned to\, so the case behind 4a2a6b3 (some-root\with a/separator) still givessome-root/.excludecallback get the output form, as before. Separators are replaced only there, and only when the output separator is not the native one (toOutputPath).a\bon POSIX) would otherwise be read as two names. Tests cover that case.One change with the default separator, where the old behaviour was wrong: on POSIX, a backslash in the root or in a resolved symlink target used to become
/. Socrawl("literal\\")readliteral/(or failed withENOENT), and a symlink todir\namereaddir/name/. Both now read the directory that exists. Relative paths that leave the root through a resolved symlink keep the backslash too.Performance, default separator, against #172's head on macOS: 300 to 600 alternating samples per version on a warm cache, crawling
node_modules(about 6k files).fs, the walker's own work is the same: 0.20 ms before and after.Under vitest's default interop, every read of an import from
pathgoes through a Proxy (about 56 ns), so measure withdeps.interopDefault: falseor the built package.Tests, each failing before and passing after, for both
syncandwithPromise:.withBasePath()and a check that it finds files (thetest.failsmarker from test: bring master's suite back to green after errors were turned on in the tests #172 is removed);excludeargument) under the other separator, plus the same forms under/to pin today's default output;a\band a directoryc\dcrawled with\; a root namedliteral\next to a siblingliteral, with either separator; symlinks (to a directory and to a file) into a directory nameddir\namenext todir/name, with real paths, link paths and relative paths, under either separator (POSIX only, skipped on Windows);resolvePathstrue and false, with and without relative paths, bothexcludecases, and recursive symlinks under bothuseRealPathssettings.The backslash cases were also checked on the real macOS file system, in the sync, promise and callback APIs.
npx vitest run: 174 passed.npx tsc --noEmitandnpx prettier --check __tests__ srcare clean.Stacked on #172, which fixes master's tests, so this branch holds #172's four commits as well until it lands. It meets #170 only at the
pathimport inresolve-symlink.ts(takeimport { dirname, normalize, sep } from "path";); with that resolved, the two together pass 176 tests.