Always use resolved file to figure out subModule name in package id #31541
Conversation
sheetalkamat
requested review from
rbuckton,
weswigham,
andrewbranch,
RyanCavanaugh and
sandersn
| let packageId: PackageId | undefined; | ||
| if (r && packageInfo) { | ||
| const packageJsonContent = packageInfo.packageJsonContent as PackageJson; | ||
| if (typeof packageJsonContent.name === "string" && typeof packageJsonContent.version === "string") { |
There was a problem hiding this comment.
name and version have type string | undefined, right? It seems a lot clearer to write packageJsonContent.name !== undefined
There was a problem hiding this comment.
If name or version aren't defined, then you get no packageId now. Before this change, was it possible to get a packageId in situations where name or version would be undefined? What happens now in that case (if it exists)?
There was a problem hiding this comment.
it was exact same condition for package id to be present .. just copied it here instead. Given that the value is coming from Json I think checking string type is better but check can be updated to isString() instead for better readability
There was a problem hiding this comment.
No, I think typeof x==='string' is just as readable as isString.
| @@ -2,11 +2,8 @@ | |||
| "======== Resolving module 'normalize.css' from '/a.ts'. ========", | |||
| "Module resolution kind is not specified, using 'NodeJs'.", | |||
| "Loading module 'normalize.css' from 'node_modules' folder, target file type 'TypeScript'.", | |||
| "'package.json' does not have a 'typings' field.", | |||
| "'package.json' does not have a 'types' field.", | |||
There was a problem hiding this comment.
Because we don’t look at the field to calculate sub module names
There was a problem hiding this comment.
Previously when sub module name was empty string these fields were read to get better name
Previously we calculated PackageId ( that is module name and submodule name) before resolving module and hence we didn't know when resolving
lib/optionwhether it is going to belib\option.d.tsfile orlib\option\index.d.tswhich resulted in us assuming first path when resolved using relative path and when resolved using non relative path respectively. So we could refer to same file but derive different package id, resulting in us assuming those as distinct path (esp if one happens to resolve in project directory and other in shared directory).Now the submodule name is always calculated using resolved file name so as to be consistent irrespective of module name being relative or non relative.
Fixes #30429