Skip to content
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

Let AutoImportProvider find non-declaration files in wildcard exports... #56848

Merged
merged 5 commits into from
Jan 3, 2024
Merged

Let AutoImportProvider find non-declaration files in wildcard exports... #56848

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec6b6e0
Let AutoImportProvider find non-declaration files in wildcard exports
andrewbranch Dec 21, 2023
437d1c7
Fix lint
andrewbranch Dec 21, 2023
d77d452
Add helper for changing full extension
andrewbranch Jan 3, 2024
775554f
Handle when `newExtension` does not start with .
andrewbranch Jan 3, 2024
a9c9548
Fix ternary/addition precedence
andrewbranch Jan 3, 2024
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
6 changes: 2 additions & 4 deletions src/compiler/moduleNameResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
appendIfUnique,
arrayIsEqualTo,
changeAnyExtension,
changeFullExtension,
CharacterCodes,
combinePaths,
CommandLineOption,
Expand Down Expand Up @@ -41,7 +42,6 @@ import {
GetCanonicalFileName,
getCommonSourceDirectory,
getCompilerOptionValue,
getDeclarationEmitExtensionForPath,
getDirectoryPath,
GetEffectiveTypeRootsHost,
getEmitModuleKind,
Expand Down Expand Up @@ -2291,9 +2291,7 @@ function loadEntrypointsFromExportMap(
extensionsToExtensionsArray(extensions),
/*excludes*/ undefined,
[
isDeclarationFileName(target)
? replaceFirstStar(target, "**/*")
: changeAnyExtension(replaceFirstStar(target, "**/*"), getDeclarationEmitExtensionForPath(target)),
changeFullExtension(replaceFirstStar(target, "**/*"), ".*"),
],
).forEach(entry => {
entrypoints = appendIfUnique(entrypoints, {
Expand Down
19 changes: 17 additions & 2 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ import {
Extension,
ExternalModuleReference,
fileExtensionIs,
fileExtensionIsOneOf,
findIndex,
firstOrUndefined,
forEach,
Expand All @@ -99,6 +98,7 @@ import {
FunctionOrConstructorTypeNode,
FunctionTypeNode,
GetAccessorDeclaration,
getAnyExtensionFromPath,
getBaseFileName,
getBinaryOperatorPrecedence,
getFullWidth,
Expand Down Expand Up @@ -10433,7 +10433,22 @@ namespace IncrementalParser {

/** @internal */
export function isDeclarationFileName(fileName: string): boolean {
return fileExtensionIsOneOf(fileName, supportedDeclarationExtensions) || (fileExtensionIs(fileName, Extension.Ts) && getBaseFileName(fileName).includes(".d."));
return getDeclarationFileExtension(fileName) !== undefined;
}

/** @internal */
export function getDeclarationFileExtension(fileName: string): string | undefined {
const standardExtension = getAnyExtensionFromPath(fileName, supportedDeclarationExtensions, /*ignoreCase*/ false);
if (standardExtension) {
return standardExtension;
}
if (fileExtensionIs(fileName, Extension.Ts)) {
const index = getBaseFileName(fileName).lastIndexOf(".d.");
if (index >= 0) {
return fileName.substring(index);
}
}
return undefined;
}

function parseResolutionMode(mode: string | undefined, pos: number, end: number, reportDiagnostic: PragmaDiagnosticReporter): ResolutionMode {
Expand Down
19 changes: 19 additions & 0 deletions src/compiler/path.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
equateStringsCaseInsensitive,
equateStringsCaseSensitive,
GetCanonicalFileName,
getDeclarationFileExtension,
getStringComparer,
identity,
lastOrUndefined,
Expand Down Expand Up @@ -756,6 +757,24 @@ export function changeAnyExtension(path: string, ext: string, extensions?: strin
return pathext ? path.slice(0, path.length - pathext.length) + (startsWith(ext, ".") ? ext : "." + ext) : path;
}

/**
* @internal
* Like `changeAnyExtension`, but declaration file extensions are recognized
* and replaced starting from the `.d`.
*
* ```ts
* changeAnyExtension("file.d.ts", ".js") === "file.d.js"
* changeFullExtension("file.d.ts", ".js") === "file.js"
* ```
*/
export function changeFullExtension(path: string, newExtension: string) {
const declarationExtension = getDeclarationFileExtension(path);
if (declarationExtension) {
return path.slice(0, path.length - declarationExtension.length) + newExtension;
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to handle presence or absence of "." in new extension like in changeAnyExtension : (startsWith(ext, ".") ? ext : "." + ext)

}
return changeAnyExtension(path, newExtension);
}

//// Path Comparisons

// check path for these segments: '', '.'. '..'
Expand Down
16 changes: 12 additions & 4 deletions tests/baselines/reference/tsserver/fourslashServer/autoImportProvider_wildcardExports1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,10 @@ Info seq [hh:mm:ss:mss] Files (4)

Info seq [hh:mm:ss:mss] -----------------------------------------------
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /package.json 250 undefined WatchType: package.json file
Info seq [hh:mm:ss:mss] AutoImportProviderProject: found 5 root files in 1 dependencies in * ms
Info seq [hh:mm:ss:mss] AutoImportProviderProject: found 6 root files in 1 dependencies in * ms
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/a/a1.d.ts 500 undefined WatchType: Closed Script info
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/b/b1.d.ts 500 undefined WatchType: Closed Script info
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/b/b2.d.mts 500 undefined WatchType: Closed Script info
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/c/c1.d.ts 500 undefined WatchType: Closed Script info
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/c/subfolder/c2.d.mts 500 undefined WatchType: Closed Script info
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/d/d1.d.mts 500 undefined WatchType: Closed Script info
Expand All @@ -183,9 +184,10 @@ Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/b/pa
Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /node_modules/pkg/c/package.json 2000 undefined Project: /dev/null/autoImportProviderProject1* WatchType: File location affecting resolution
Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /dev/null/autoImportProviderProject1* Version: 1 structureChanged: true structureIsReused:: Not Elapsed:: *ms
Info seq [hh:mm:ss:mss] Project '/dev/null/autoImportProviderProject1*' (AutoImportProvider)
Info seq [hh:mm:ss:mss] Files (5)
Info seq [hh:mm:ss:mss] Files (6)
/node_modules/pkg/a/a1.d.ts Text-1 "export const a1: number;"
/node_modules/pkg/b/b1.d.ts Text-1 "export const b1: number;"
/node_modules/pkg/b/b2.d.mts Text-1 "export const NOT_REACHABLE: number;"
Comment on lines -186 to +190
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I could have kept the same strict extension filtering while mixing in the allowed TS implementation file extensions, keeping this unreachable file out of the AutoImportProviderProject compilation. But it doesn’t seem realistic for packages to ship declaration files that are literally unreachable, so doing the simpler thing and allowing all supported extensions seems fine. Updated the test to show that this file doesn’t result in a new auto-import.

/node_modules/pkg/c/c1.d.ts Text-1 "export const c1: number;"
/node_modules/pkg/c/subfolder/c2.d.mts Text-1 "export const c2: number;"
/node_modules/pkg/d/d1.d.mts Text-1 "export const d1: number;"
Expand All @@ -197,6 +199,8 @@ Info seq [hh:mm:ss:mss] Files (5)
node_modules/pkg/b/b1.d.ts
Root file specified for compilation
File is CommonJS module because 'node_modules/pkg/package.json' does not have field "type"
node_modules/pkg/b/b2.d.mts
Root file specified for compilation
node_modules/pkg/c/c1.d.ts
Root file specified for compilation
File is CommonJS module because 'node_modules/pkg/package.json' does not have field "type"
Expand Down Expand Up @@ -231,7 +235,7 @@ Info seq [hh:mm:ss:mss] Files (4)

Info seq [hh:mm:ss:mss] -----------------------------------------------
Info seq [hh:mm:ss:mss] Project '/dev/null/autoImportProviderProject1*' (AutoImportProvider)
Info seq [hh:mm:ss:mss] Files (5)
Info seq [hh:mm:ss:mss] Files (6)

Info seq [hh:mm:ss:mss] -----------------------------------------------
Info seq [hh:mm:ss:mss] Project '/dev/null/inferredProject1*' (Inferred)
Expand All @@ -257,6 +261,8 @@ watchedFiles::
{"pollingInterval":2000}
/node_modules/pkg/b/b1.d.ts: *new*
{"pollingInterval":500}
/node_modules/pkg/b/b2.d.mts: *new*
{"pollingInterval":500}
/node_modules/pkg/b/package.json: *new*
{"pollingInterval":2000}
/node_modules/pkg/c/c1.d.ts: *new*
Expand Down Expand Up @@ -315,7 +321,7 @@ Info seq [hh:mm:ss:mss] forEachExternalModuleToImportFrom autoImportProvider: *
Info seq [hh:mm:ss:mss] getExportInfoMap: done in * ms
Info seq [hh:mm:ss:mss] DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
Info seq [hh:mm:ss:mss] collectAutoImports: resolved 5 module specifiers, plus 0 ambient and 0 from cache
Info seq [hh:mm:ss:mss] collectAutoImports: resolved 5 module specifiers, plus 0 ambient and 1 from cache
Info seq [hh:mm:ss:mss] collectAutoImports: response is complete
Info seq [hh:mm:ss:mss] collectAutoImports: *
Info seq [hh:mm:ss:mss] getCompletionData: Semantic work: *
Expand Down Expand Up @@ -1108,6 +1114,8 @@ watchedFiles::
{"pollingInterval":2000}
/node_modules/pkg/b/b1.d.ts:
{"pollingInterval":500}
/node_modules/pkg/b/b2.d.mts:
{"pollingInterval":500}
/node_modules/pkg/b/package.json:
{"pollingInterval":2000}
/node_modules/pkg/c/c1.d.ts:
Expand Down
Loading
Loading