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

Add additional diagnostic for packages that only resolve under non-exports-respecting modes #52173

Conversation

andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented Jan 9, 2023

Adds a message to most of the implicit any errors in #51973 (you can’t see the chained details on the bot’s GitHub comment, unfortunately)

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 9, 2023
@andrewbranch andrewbranch marked this pull request as ready for review January 11, 2023 00:09
src/compiler/types.ts Outdated Show resolved Hide resolved
@@ -5235,6 +5235,14 @@
"category": "Message",
"code": 6276
},
"Resolution of non-relative name failed; trying with modern Node resolution features disabled to see if npm library needs configuration update.": {
Copy link
Member

Choose a reason for hiding this comment

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

a non-relative name is one like "foo", not "./foo.js", right?

I guess this is output for traceResolution, so it's probably OK to be technical, but is 'non-relative' the friendliest term we have? I have to think about it a bit every time I see the term. Could be that I was ruined by reading an old, old "how Typescript implements Node resolution" document, though.

Choose a reason for hiding this comment

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

I've also found myself wondering if there's a good term for imports that are neither absolute nor relative to the containing file, but rather relative to some configured root or resolved via aliasing. Floating? Inferred? I guess "absolute" could be reused since giving full filesystem paths for imports should be vanishingly rare, but it doesn't feel accurate either.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I like the new error a lot better. I had a couple of bike-shed comments but no suggestions that I am confident in.

"category": "Message",
"code": 6277
},
"There are types at '{0}', but this result could not be resolved when respecting package.json \"exports\". The '{1}' library may need to update its 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.

Q: Would a similar error for "imports" for resolving #-prefixed nonrelative names be useful?

@andrewbranch andrewbranch merged commit 3e9703f into microsoft:main Jan 13, 2023
@andrewbranch andrewbranch deleted the module-resolution/esm-types-resolution-diagnostic branch January 13, 2023 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants