Skip to content

Commit 7480024

Browse files
committed
Support nested conditions, improve recursive globbing
1 parent ed77c7c commit 7480024

File tree

3 files changed

+115
-14
lines changed

3 files changed

+115
-14
lines changed

src/services/stringCompletions.ts

+18-14
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ namespace ts.Completions.StringCompletions {
711711
extensionOptions,
712712
host,
713713
keys,
714-
key => getPatternFromFirstMatchingCondition(exports[key], conditions),
714+
key => singleElementArray(getPatternFromFirstMatchingCondition(exports[key], conditions)),
715715
comparePatternKeys);
716716
return;
717717
}
@@ -726,15 +726,15 @@ namespace ts.Completions.StringCompletions {
726726
return arrayFrom(result.values());
727727
}
728728

729-
function getPatternFromFirstMatchingCondition(target: unknown, conditions: readonly string[]): [string] | undefined {
729+
function getPatternFromFirstMatchingCondition(target: unknown, conditions: readonly string[]): string | undefined {
730730
if (typeof target === "string") {
731-
return [target];
731+
return target;
732732
}
733733
if (target && typeof target === "object" && !isArray(target)) {
734734
for (const condition in target) {
735735
if (condition === "default" || conditions.indexOf(condition) > -1 || isApplicableVersionedTypesKey(conditions, condition)) {
736736
const pattern = (target as MapLike<unknown>)[condition];
737-
return typeof pattern === "string" ? [pattern] : undefined;
737+
return getPatternFromFirstMatchingCondition(pattern, conditions);
738738
}
739739
}
740740
}
@@ -804,27 +804,31 @@ namespace ts.Completions.StringCompletions {
804804
const baseDirectory = normalizePath(combinePaths(packageDirectory, expandedPrefixDirectory));
805805
const completePrefix = fragmentHasPath ? baseDirectory : ensureTrailingDirectorySeparator(baseDirectory) + normalizedPrefixBase;
806806

807-
// If we have a suffix of at least a whole filename (i.e., not just an extension), then we need
808-
// to read the directory all the way down to see which directories contain a file matching that suffix.
809-
const suffixHasFilename = normalizedSuffix && containsSlash(normalizedSuffix);
810-
const includeGlob = suffixHasFilename ? "**/*" : "./*";
807+
// If we have a suffix, then we read the directory all the way down to avoid returning completions for
808+
// directories that don't contain files that would match the suffix. A previous comment here was concerned
809+
// about the case where `normalizedSuffix` includes a `?` character, which should be interpreted literally,
810+
// but will match any single character as part of the `include` pattern in `tryReadDirectory`. This is not
811+
// a problem, because (in the extremely unusual circumstance where the suffix has a `?` in it) a `?`
812+
// interpreted as "any character" can only return *too many* results as compared to the literal
813+
// interpretation, so we can filter those superfluous results out via `trimPrefixAndSuffix` as we've always
814+
// done.
815+
const includeGlob = normalizedSuffix ? "**/*" + normalizedSuffix : "./*";
811816

812817
const matches = mapDefined(tryReadDirectory(host, baseDirectory, extensionOptions.extensions, /*exclude*/ undefined, [includeGlob]), match => {
813818
const trimmedWithPattern = trimPrefixAndSuffix(match);
814819
if (trimmedWithPattern) {
815820
if (containsSlash(trimmedWithPattern)) {
816-
return directoryResult(getPathComponents(trimmedWithPattern)[0]);
821+
return directoryResult(getPathComponents(removeLeadingDirectorySeparator(trimmedWithPattern))[1]);
817822
}
818823
const { name, extension } = getFilenameWithExtensionOption(trimmedWithPattern, host.getCompilationSettings(), extensionOptions.includeExtensionsOption);
819824
return nameAndKind(name, ScriptElementKind.scriptElement, extension);
820825
}
821826
});
822827

823-
// If the suffix had a whole filename in it, we already recursively searched for all possible files
824-
// that could match it and returned the directories that could possibly lead to matches. Otherwise,
825-
// assume any directory could have something valid to import (not a great assumption - we could
826-
// consider doing a whole glob more often and caching the results?)
827-
const directories = suffixHasFilename
828+
// If we had a suffix, we already recursively searched for all possible files that could match
829+
// it and returned the directories leading to those files. Otherwise, assume any directory could
830+
// have something valid to import.
831+
const directories = normalizedSuffix
828832
? emptyArray
829833
: mapDefined(tryGetDirectories(host, baseDirectory), dir => dir === "node_modules" ? undefined : directoryResult(dir));
830834
return [...matches, ...directories];
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @module: nodenext
4+
5+
// @Filename: /node_modules/foo/package.json
6+
//// {
7+
//// "name": "foo",
8+
//// "main": "dist/index.js",
9+
//// "module": "dist/index.mjs",
10+
//// "types": "dist/index.d.ts",
11+
//// "exports": {
12+
//// ".": {
13+
//// "import": {
14+
//// "types": "./dist/types/index.d.mts",
15+
//// "default": "./dist/esm/index.mjs"
16+
//// },
17+
//// "default": {
18+
//// "types": "./dist/types/index.d.ts",
19+
//// "default": "./dist/cjs/index.js"
20+
//// }
21+
//// },
22+
//// "./*": {
23+
//// "import": {
24+
//// "types": "./dist/types/*.d.mts",
25+
//// "default": "./dist/esm/*.mjs"
26+
//// },
27+
//// "default": {
28+
//// "types": "./dist/types/*.d.ts",
29+
//// "default": "./dist/cjs/*.js"
30+
//// }
31+
//// },
32+
//// "./only-in-cjs": {
33+
//// "require": {
34+
//// "types": "./dist/types/only-in-cjs/index.d.ts",
35+
//// "default": "./dist/cjs/only-in-cjs/index.js"
36+
//// }
37+
//// }
38+
//// }
39+
//// }
40+
41+
// @Filename: /node_modules/foo/dist/types/index.d.mts
42+
//// export const index = 0;
43+
44+
// @Filename: /node_modules/foo/dist/types/index.d.ts
45+
//// export const index = 0;
46+
47+
// @Filename: /node_modules/foo/dist/types/blah.d.mts
48+
//// export const blah = 0;
49+
50+
// @Filename: /node_modules/foo/dist/types/blah.d.ts
51+
//// export const blah = 0;
52+
53+
// @Filename: /node_modules/foo/dist/types/only-in-cjs/index.d.ts
54+
//// export const onlyInCjs = 0;
55+
56+
// @Filename: /index.mts
57+
//// import { } from "foo//**/";
58+
59+
verify.completions({
60+
marker: "",
61+
isNewIdentifierLocation: true,
62+
exact: [
63+
{ name: "blah", kind: "script", kindModifiers: "" },
64+
{ name: "index", kind: "script", kindModifiers: "" },
65+
]
66+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @module: nodenext
4+
5+
// @Filename: /node_modules/foo/package.json
6+
//// {
7+
//// "name": "foo",
8+
//// "main": "dist/index.js",
9+
//// "module": "dist/index.mjs",
10+
//// "types": "dist/index.d.ts",
11+
//// "exports": {
12+
//// "./*": "./dist/*?.d.ts"
13+
//// }
14+
//// }
15+
16+
// @Filename: /node_modules/foo/dist/index.d.ts
17+
//// export const index = 0;
18+
19+
// @Filename: /node_modules/foo/dist/blah?.d.ts
20+
//// export const blah = 0;
21+
22+
// @Filename: /index.mts
23+
//// import { } from "foo//**/";
24+
25+
verify.completions({
26+
marker: "",
27+
isNewIdentifierLocation: true,
28+
exact: [
29+
{ name: "blah", kind: "script", kindModifiers: "" },
30+
]
31+
});

0 commit comments

Comments
 (0)