-
Notifications
You must be signed in to change notification settings - Fork 5.9k
fix(check): improve @types/ package resolution for the global resolver
#31868
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
fix(check): improve @types/ package resolution for the global resolver
#31868
Conversation
@types/ package resolution for the global resolver
| let (_, nv) = find_definitely_typed_package( | ||
| types_package_name, | ||
| maybe_package_version, | ||
| snapshot.package_reqs().iter(), |
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.
Previously we were only looking at top level npm packages, which would mean we wouldn't resolve a whole lot of @types packages. This bug fix should fix a lot of type checking issues with the global resolver.
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.
It's better to add a real integration test for this one.
| let resolved = deno_path_util::resolve_import(&specifier, referrer)?; | ||
| let ext = MediaType::from_specifier(&resolved).as_ts_extension(); | ||
| return Ok((specifier, Some(ext))); | ||
| } |
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.
We accidentally weren't resolving some of the files for the built-in @types/node package.
nathanwhit
left a comment
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.
LGTM!
|
So cool! |
Closes #31697