Repository navigation
Keep the cwd out of glob patterns and matchers - #2107
Conversation
webpro
left a comment
There was a problem hiding this comment.
Thanks for the pull request! Knip should just work from a directory like ~/Dropbox (Team)/app, and this fixes the reported case. Running the test suite from a checkout in such a directory, 45 tests still fail with this change, for example in ignore patterns, production mode (Vitest, node-test-runner), subpath imports, tsconfig include and import.meta.glob. Could you make the existing tests pass from such a directory, and add tests that cover these cases? One more note inline.
| const toCwdRelative = (pattern: string, cwd: string) => | ||
| pattern.startsWith('!') ? `!${toRelative(pattern.slice(1), cwd)}` : toRelative(pattern, cwd); | ||
|
|
||
| export async function glob(_patterns: string[], options: GlobOptions): Promise<string[]> { | ||
| if (Array.isArray(_patterns) && _patterns.length === 0) return []; | ||
|
|
||
| const cachedIgnores = options.gitignore ? cachedGlobIgnores.get(options.dir) : undefined; | ||
|
|
||
| const _ignore: string[] = [...GLOBAL_IGNORE_PATTERNS]; | ||
| const [negatedPatterns, patterns] = partition(_patterns, pattern => pattern.startsWith('!')); | ||
| const [negatedPatterns, patterns] = partition( | ||
| _patterns.map(pattern => toCwdRelative(pattern, options.cwd)), |
There was a problem hiding this comment.
tinyglobby relativizes absolute patterns against the escaped cwd, so the cwd does need to come out of the patterns on our side. But this adds another pass over every pattern with its own ! handling. prependDirToPatterns in glob.ts already maps every pattern and checks for absolute ones, so the conversion fits in there, for example:
const prepend = (pattern: string, cwd: string, relativePath: string) => {
const isNegated = pattern.startsWith('!');
const id = isNegated ? pattern.slice(1) : pattern;
const globPattern = isAbsolute(id) ? relative(cwd, id) : join(relativePath, id);
return isNegated ? `!${globPattern}` : globPattern;
};With that (and cwd passed in from prependDirToPatterns) instead of the change here, the new test passes, and so does every test that passes with this PR.
There was a problem hiding this comment.
Done in f0e99ff: the conversion now lives in prependDirToPatterns as you suggested, and the glob-core.ts change is gone. _syncGlob and _dirGlob use the same prepend, since tsconfig include, subpath imports and import.meta.glob reach tinyglobby through _syncGlob with absolute patterns.
The cwd may contain glob characters, as in `~/Dropbox (Team)/app`.
Absolute patterns that repeat it match nothing, so make them relative
to the cwd before globbing (in `prependDirToPatterns`, `_syncGlob` and
`_dirGlob`), and match ignore, production and configuration hint
patterns against cwd-relative paths, like the gitignore matcher does.
Nx and moonrepo expand `{workspaceRoot}`/`$workspaceRoot` into commands
that are parsed as shell scripts, so expand them to relative paths.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2304c47 to
f0e99ff
Compare
|
Thanks for the review and for running it from such a directory. I reproduced it with a checkout under |
An empty pattern used to become `.` for the root workspace and match nothing; now that relative patterns pass through untouched it is dropped, which left `defaultGlob` dereferencing the first of zero patterns.
|
I pushed a few commits on top of yours that keep the fix but stop converting the same paths back and forth: the glob layer relativizes the workspace dir once per call and only rewrites absolute patterns, the configuration hints relativize file paths only up to the first match, package.json entry specifiers are no longer made absolute just for the glob, and tsconfig Could you give it a final spin from your directory with glob characters? |
commit: |
|
Ran the full suite at 194458c from |
webpro
left a comment
There was a problem hiding this comment.
Thanks for the confirmation! Much appreciated.
|
🚀 This pull request is included in v6.41.0. See Release 6.41.0 for release notes. Using Knip in a commercial project? Please consider becoming a sponsor. |
When the project lives in a directory with glob characters, like
~/Dropbox (Team)/app, absolute patterns repeat the cwd and match nothing: plugin config files aren't loaded,ignoreand production exclusions miss, and subpath imports, tsconfigincludeandimport.meta.globresolve to nothing.prependDirToPatterns, plus_syncGloband_dirGlob).ignore,ignoreFiles,ignoreIssues, production and configuration hint patterns are matched against cwd-relative paths, like the gitignore matcher already does.includefrom the relative pattern (it was cut at the(and globbed from a parent dir).{workspaceRoot}/$workspaceRootto relative paths: the commands are parsed as shell scripts, and the space and parentheses in the absolute path broke them.test/infra/glob-characters-in-cwd.test.tscopies 12 fixtures covering these cases intoknip (fixture) [x]-…and expects the same issues, counters and hints as in their original location; all 12 fail without the change. From a checkout underCoding(Real)/, the full suite now gives the same results as from a plain directory (42 tests failed only there before).