Skip to content

Cache parsed path mapping patterns #44078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 26, 2021
Merged

Cache parsed path mapping patterns #44078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b945b9b
Cache parsed path mapping patterns
amcasey May 13, 2021
00b9ccd
Move cache to ConfigFileSpecs
amcasey May 19, 2021
ac0c9aa
Inline constants
amcasey May 25, 2021
0435303
Simplify cache access
amcasey May 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1977,19 +1977,17 @@ namespace ts {
declareModuleSymbol(node);
}
else {
let pattern: Pattern | undefined;
let pattern: string | Pattern | undefined;
if (node.name.kind === SyntaxKind.StringLiteral) {
const { text } = node.name;
if (hasZeroOrOneAsteriskCharacter(text)) {
pattern = tryParsePattern(text);
}
else {
pattern = tryParsePattern(text);
if (pattern === undefined) {
errorOnFirstToken(node.name, Diagnostics.Pattern_0_can_have_at_most_one_Asterisk_character, text);
}
}

const symbol = declareSymbolAndAddToSymbolTable(node, SymbolFlags.ValueModule, SymbolFlags.ValueModuleExcludes)!;
file.patternAmbientModules = append<PatternAmbientModule>(file.patternAmbientModules, pattern && { pattern, symbol });
file.patternAmbientModules = append<PatternAmbientModule>(file.patternAmbientModules, pattern && !isString(pattern) ? { pattern, symbol } : undefined);
}
}
else {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2533,6 +2533,7 @@ namespace ts {
validatedFilesSpec: filter(filesSpecs, isString),
validatedIncludeSpecs,
validatedExcludeSpecs,
pathPatterns: undefined, // Initialized on first use
};
}

Expand Down
14 changes: 8 additions & 6 deletions src/compiler/moduleNameResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ namespace ts {
}

function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: string, loader: ResolutionKindSpecificLoader, state: ModuleResolutionState) {
const { baseUrl, paths } = state.compilerOptions;
const { baseUrl, paths, configFile } = state.compilerOptions;
if (paths && !pathIsRelative(moduleName)) {
if (state.traceEnabled) {
if (baseUrl) {
Expand All @@ -961,7 +961,8 @@ namespace ts {
trace(state.host, Diagnostics.paths_option_is_specified_looking_for_a_pattern_to_match_module_name_0, moduleName);
}
const baseDirectory = getPathsBasePath(state.compilerOptions, state.host)!; // Always defined when 'paths' is defined
return tryLoadModuleUsingPaths(extensions, moduleName, baseDirectory, paths, loader, /*onlyRecordFailures*/ false, state);
const pathPatterns = configFile?.configFileSpecs ? configFile.configFileSpecs.pathPatterns ||= tryParsePatterns(paths) : undefined;
return tryLoadModuleUsingPaths(extensions, moduleName, baseDirectory, paths, pathPatterns, loader, /*onlyRecordFailures*/ false, state);
}
}

Expand Down Expand Up @@ -1400,7 +1401,7 @@ namespace ts {
if (state.traceEnabled) {
trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, versionPaths.version, version, moduleName);
}
const result = tryLoadModuleUsingPaths(extensions, moduleName, candidate, versionPaths.paths, loader, onlyRecordFailuresForPackageFile || onlyRecordFailuresForIndex, state);
const result = tryLoadModuleUsingPaths(extensions, moduleName, candidate, versionPaths.paths, /*pathPatterns*/ undefined, loader, onlyRecordFailuresForPackageFile || onlyRecordFailuresForIndex, state);
if (result) {
return removeIgnoredPackageId(result.value);
}
Expand Down Expand Up @@ -1536,7 +1537,7 @@ namespace ts {
trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, packageInfo.versionPaths.version, version, rest);
}
const packageDirectoryExists = nodeModulesDirectoryExists && directoryProbablyExists(packageDirectory, state.host);
const fromPaths = tryLoadModuleUsingPaths(extensions, rest, packageDirectory, packageInfo.versionPaths.paths, loader, !packageDirectoryExists, state);
const fromPaths = tryLoadModuleUsingPaths(extensions, rest, packageDirectory, packageInfo.versionPaths.paths, /*pathPatterns*/ undefined, loader, !packageDirectoryExists, state);
if (fromPaths) {
return fromPaths.value;
}
Expand All @@ -1546,8 +1547,9 @@ namespace ts {
return loader(extensions, candidate, !nodeModulesDirectoryExists, state);
}

function tryLoadModuleUsingPaths(extensions: Extensions, moduleName: string, baseDirectory: string, paths: MapLike<string[]>, loader: ResolutionKindSpecificLoader, onlyRecordFailures: boolean, state: ModuleResolutionState): SearchResult<Resolved> {
const matchedPattern = matchPatternOrExact(getOwnKeys(paths), moduleName);
function tryLoadModuleUsingPaths(extensions: Extensions, moduleName: string, baseDirectory: string, paths: MapLike<string[]>, pathPatterns: readonly (string | Pattern)[] | undefined, loader: ResolutionKindSpecificLoader, onlyRecordFailures: boolean, state: ModuleResolutionState): SearchResult<Resolved> {
pathPatterns ||= tryParsePatterns(paths);
const matchedPattern = matchPatternOrExact(pathPatterns, moduleName);
if (matchedPattern) {
const matchedStar = isString(matchedPattern) ? undefined : matchedText(matchedPattern, moduleName);
const matchedPatternText = isString(matchedPattern) ? matchedPattern : patternText(matchedPattern);
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6195,6 +6195,7 @@ namespace ts {
validatedFilesSpec: readonly string[] | undefined;
validatedIncludeSpecs: readonly string[] | undefined;
validatedExcludeSpecs: readonly string[] | undefined;
pathPatterns: readonly (string | Pattern)[] | undefined;
}

/* @internal */
Expand Down
45 changes: 27 additions & 18 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6769,14 +6769,25 @@ namespace ts {
return changeAnyExtension(path, newExtension, extensionsToRemove, /*ignoreCase*/ false) as T;
}

export function tryParsePattern(pattern: string): Pattern | undefined {
// This should be verified outside of here and a proper error thrown.
Debug.assert(hasZeroOrOneAsteriskCharacter(pattern));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we remove the assert or requirement that users of this have this checked already?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check was duplicated between the caller and the assert (none needed it for other reasons - they were just doing it to satisfy this requirement). I didn't think the requirement made the API conceptually clearer or the implementation simpler - on the contrary, I thought it was preferable to gather all the checks into a single function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps your question was "what happened to the proper error?". It's still there - an undefined result now indicates that an error should be reported.

/**
* Returns the input if there are no stars, a pattern if there is exactly one,
* and undefined if there are more.
*/
export function tryParsePattern(pattern: string): string | Pattern | undefined {
const indexOfStar = pattern.indexOf("*");
return indexOfStar === -1 ? undefined : {
prefix: pattern.substr(0, indexOfStar),
suffix: pattern.substr(indexOfStar + 1)
};
if (indexOfStar === -1) {
return pattern;
}
return pattern.indexOf("*", indexOfStar + 1) !== -1
? undefined
: {
prefix: pattern.substr(0, indexOfStar),
suffix: pattern.substr(indexOfStar + 1)
};
}

export function tryParsePatterns(paths: MapLike<string[]>): (string | Pattern)[] {
return mapDefined(getOwnKeys(paths), path => tryParsePattern(path));
}

export function positionIsSynthesized(pos: number): boolean {
Expand Down Expand Up @@ -6822,21 +6833,19 @@ namespace ts {


/**
* patternStrings contains both pattern strings (containing "*") and regular strings.
* patternOrStrings contains both patterns (containing "*") and regular strings.
* Return an exact match if possible, or a pattern match, or undefined.
* (These are verified by verifyCompilerOptions to have 0 or 1 "*" characters.)
*/
export function matchPatternOrExact(patternStrings: readonly string[], candidate: string): string | Pattern | undefined {
export function matchPatternOrExact(patternOrStrings: readonly (string | Pattern)[], candidate: string): string | Pattern | undefined {
const patterns: Pattern[] = [];
for (const patternString of patternStrings) {
if (!hasZeroOrOneAsteriskCharacter(patternString)) continue;
const pattern = tryParsePattern(patternString);
if (pattern) {
patterns.push(pattern);
}
else if (patternString === candidate) {
// pattern was matched as is - no need to search further
return patternString;
for (const patternOrString of patternOrStrings) {
if (patternOrString === candidate) {
return candidate;
}

if (!isString(patternOrString)) {
patterns.push(patternOrString);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/services/stringCompletions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,8 @@ namespace ts.Completions.StringCompletions {
return undefined;
}

const parsed = hasZeroOrOneAsteriskCharacter(pattern) ? tryParsePattern(pattern) : undefined;
if (!parsed) {
const parsed = tryParsePattern(pattern);
if (parsed === undefined || isString(parsed)) {
return undefined;
}

Expand Down