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

Update the reason for not fixing types #122

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

h-joo
Copy link

@h-joo h-joo commented Nov 28, 2023

These type originate from node_modules and as the compiler refuses to create proper type node for them as it would be adding an import to node_modules. We also don't do anything about it.

These type originate from node_modules and as the compiler
refuses to create proper type node for them as it would be adding an
import to node_modules. We also don't do anything about it.

Signed-off-by: Hana Joo <hanajoo@google.com>
@dragomirtitian
Copy link

@h-joo that is very interesting. Any idea why it fails? Is it hardcoded to do that for node_modules?

@h-joo
Copy link
Author

h-joo commented Nov 29, 2023

@h-joo that is very interesting. Any idea why it fails? Is it hardcoded to do that for node_modules?

Yes, if you're curious, you can look for a comment containing node_modules in checker.ts.
It says :

// If ultimately we can only name the symbol with a reference that dives into a `node_modules` folder, we should error
// since declaration files with these kinds of references are liable to fail when published :(

@dragomirtitian dragomirtitian merged commit 4e01096 into bloomberg:isolated-declarations Nov 29, 2023
1 check passed
@dragomirtitian
Copy link

Intresting. For example in [this](https://github.com/bloomberg/TypeScript/blob/92b3dcb1a2eba2750624b585bc9f905305883ba0/tests/cases/conformance/node/nodeModulesExportsBlocksSpecifierResolution.ts) test, ashould beThing` in my opinion. And there is already an import for that. So, this still seems suspect to me, but this was definitely an existing behavior.

Improving the type printing algorithm is not in our scope so it's fine to just put that as te reason for the failure.

@h-joo h-joo deleted the fixerIssues branch February 19, 2024 16:06
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.

2 participants