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.

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(node): resolve types via package.json for directory import #22878

Merged
merged 6 commits into from
Mar 14, 2024

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Mar 12, 2024

Does a package resolve when resolving types for a directory (copying the behaviour that typescript does).

Please merge on approval

@dsherret dsherret marked this pull request as ready for review March 13, 2024 19:13
if let Some(resolution) = maybe_resolution {
return Ok(Some(resolution));
}
}
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 looked into it and this seems to be done first before the index.d.ts probing.

@dsherret dsherret changed the title fix(node): fix preact/hooks types fix(node): resolve types via package.json for directory import Mar 13, 2024
Comment on lines -514 to -518
throw new ERR_MODULE_NOT_FOUND(
filename,
path.resolve(pkgPath, "package.json"),
);
}
Copy link
Member

Choose a reason for hiding this comment

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

IIRC this was quite an important bit and some packages were checking for this ERR_MODULE_NOT_FOUND code property. Does this value still match after this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

This code was not used so I removed it.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, let's land as is, but just a warning for the future. I'm not 100% sure this was the exact code path that was checking for this code.

@dsherret dsherret merged commit bc782ce into denoland:main Mar 14, 2024
17 checks passed
@dsherret dsherret deleted the fix_directory_resolution_error branch March 14, 2024 02:37
nathanwhit pushed a commit that referenced this pull request Mar 14, 2024
Does a package resolve when resolving types for a directory (copying the
behaviour that typescript does).
dsherret added a commit to dsherret/deno that referenced this pull request Mar 15, 2024
…and#22878)

Does a package resolve when resolving types for a directory (copying the
behaviour that typescript does).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants