-
Notifications
You must be signed in to change notification settings - Fork 648
Port IsSourceFileFromExternalLibrary so that we can implement more paths in sourceFileMayBeEmitted
#1234
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.
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
base: main
Are you sure you want to change the base?
Conversation
@@ -970,6 +970,7 @@ func (p *Project) GetFileNames(excludeFilesFromExternalLibraries bool, excludeCo | |||
result := []string{} | |||
sourceFiles := p.program.GetSourceFiles() | |||
for _, sourceFile := range sourceFiles { | |||
// !! This is probably ready to be implemented now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we have IsSourceFileFromExternalLibrary
// IsSourceFileFromExternalLibrary returns true if the source file is from an external library. | ||
// This mirrors the TypeScript implementation which tracks files found while searching node_modules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume some LLM added the comments; I would just remove all of these as they just describe exactly what the code says.
fileName: resolvedFileName, | ||
increaseDepth: resolvedModule.IsExternalLibraryImport, | ||
elideOnDepth: isJsFileFromNodeModules, | ||
isFromExternalLibrary: isFromNodeModulesSearch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just resolvedModule.IsExternalLibraryImport
?
increaseDepth: false, | ||
elideOnDepth: false, | ||
isFromExternalLibrary: false, // Triple-slash references are always local files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all defaulted to false
if not present.
isExternal, exists := p.sourceFilesFoundSearchingNodeModules.Load(file.Path()) | ||
return exists && isExternal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should have Load
. It's just a map
.
@@ -50,6 +50,9 @@ type processedFiles struct { | |||
importHelpersImportSpecifiers map[tspath.Path]*ast.Node | |||
// List of present unsupported extensions | |||
unsupportedExtensions []string | |||
// Track which source files were found while searching node_modules | |||
// Similar to TypeScript's sourceFilesFoundSearchingNodeModules map | |||
sourceFilesFoundSearchingNodeModules map[tspath.Path]bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use collections.Set[tspath.Path]
here.
if p.processedFiles.sourceFilesFoundSearchingNodeModules != nil { | ||
for path, isExternal := range p.processedFiles.sourceFilesFoundSearchingNodeModules { | ||
p.sourceFilesFoundSearchingNodeModules.Store(path, isExternal) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be required, the info should already be on processedFiles and therefore be accessible.
and close
.ts
alongside.js
of the same name, builds are nested #1215