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

Giving an error type to types that have failed to resolve, instead of any #61089

Open
6 tasks done
RobertSandiford opened this issue Jan 31, 2025 · 4 comments
Open
6 tasks done
Labels
Experimentation Needed Someone needs to try this out to see what happens Suggestion An idea for TypeScript
Milestone

Comments

@RobertSandiford
Copy link

πŸ” Search Terms

Types that fail to import should not be typed as any. SkipLibCheck. Type resolution failure.

βœ… Viability Checklist

⭐ Suggestion

Currently if TS fails to resolve a type, for example because it can't resolve an import, the type is set as any. TS will display the type by it's alias (the local name given to the type), hiding that fact that it is any underneath. any being a special type that largely disables type checking, being silently introduced, can hide code errors.

Failing imports are typically reported by TS as an error, even if using that any typed type later on doesn't produce an error. However if there is a type resolution failure within an external module with skipLibCheck turned on (which is widely used and recommended, for multiple reasons), then there is no error and an any type is silently introduced to the project types.

The suggested solution is instead type these types with an error type, working name: failed. An assignment to or from a failed type is an error, like an opposite to any.

--

I've ticked non-breaking, even though it could lead to compilation of existing code failing, because it would only be highlighting an error that previously wasn't highlighted. It's non-breaking in valid code.

There would be a degree of annoyance never-the-less in triggering compilation errors in code that previously compiled, caused by problems in external libraries. Error messages indicating where the type resolution failed would be very helpful here.

I'm putting this out for discussion, because it's difficult for me to foresee the full consequences and challenges of such a change.

πŸ“ƒ Motivating Example

In this simple Playground we import a type from a package with an internal resolution error, that leads a type X['c'] to be any. TS reports no errors to us and lets us perform type-irrational operations.

πŸ’» Use Cases

Fairly obviously, it's a safeguard against mistakes in external libraries, and it would also make broken imports more obvious by showing errors at the use site not just at the import site.

This occurred to me when dealing with a typing issue in @stripe/stripe-js (1.3M weekly downloads) stripe-js#714. Just fixing the import in the index file as the maintainer suggested would have introduced this issue to that library.

@MartinJohns
Copy link
Contributor

Related: #54630

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experimentation Needed Someone needs to try this out to see what happens labels Jan 31, 2025
@RyanCavanaugh
Copy link
Member

it's difficult for me to foresee the full consequences and challenges of such a change.

Honestly, same. I think a rough implementation sketch would be that failed imports do generate some kind of special failed type (with per-instance info of which resolution failed), and that all type constructor operations (keyof, conditional type resolution, indexed access, etc) propagate that failed state. We could error immediately on the import if you write

import { IsTopLevelFailed } from "whatever";

However, my real concerns are how this would work at a meta level. People seeing this type will immediately try to leverage it to add behavior like "If I can see the module "react", then do X, otherwise do Y" and get frustrated when there's intentionally no mechanism to do that, which would probably lead to demand for an IsFailed operator or something, then it's turtles all the way down where there's demand for a "no really, don't let people do that" flag, etc etc.

It would be interesting to see what happens with the first stage of this, though. We could run it on the top800 repos and see if anyone is unintentionally having anys creep in - that's a useful signal for further evaluation.

Re stripe-js, it's really important that anyone hand-authoring or publishing a .d.ts file turn off skipLibCheck as part of their build check. This is something we need to figure out a better story for.

@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jan 31, 2025
@RobertSandiford
Copy link
Author

RobertSandiford commented Jan 31, 2025

However, my real concerns are how this would work at a meta level. People seeing this type will immediately try to leverage it to add behavior like "If I can see the module "react", then do X, otherwise do Y" and get frustrated when there's intentionally no mechanism to do that, which would probably lead to demand for an IsFailed operator or something, then it's turtles all the way down where there's demand for a "no really, don't let people do that" flag, etc etc.

Yeah I mean the failed type is just a way to track that there has been a failure and providing reporting info. Maybe it's wrong to call it a type, it's more about reporting the error. My thinking is that storing the failure as "type" internally with error source information (the type of the value or type alias being an object of the kind 'failed' with failed location information) would be an easy way to transit that information to where that failed type is used in the project and then report the information. But perhaps don't call it a type to users, it's just an error report to them. The original error could be displayed when a failed type is used in a project, meaning that we are effectively showing specific lib errors that relate to our current project while filtering out the rest. I'm not sure how much error information we have in the case that skipLibCheck is on - clearly we are still reading those lib files, but perhaps we're not saving the diagnostics on errors?

It would be interesting to see what happens with the first stage of this, though. We could run it on the top800 repos and see if anyone is unintentionally having anys creep in - that's a useful signal for further evaluation.

Sure

Re stripe-js, it's really important that anyone hand-authoring or publishing a .d.ts file turn off skipLibCheck as part of their build check. This is something we need to figure out a better story for.

I think what most TS devs want is a flag to turn off error reporting of errors in other people's .d.ts files (node_modules), (except perhaps where they are relevant to our project like in this case), but keep checking .d.ts files in the current project. I.e. "skipExternalLibCheck". That would be an easy setting of choice for most people.

@RobertSandiford
Copy link
Author

RobertSandiford commented Jan 31, 2025

I'm hearing that TS does have an internal error type already, just that it is presented to the user as being any. So more an issue on the reporting side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experimentation Needed Someone needs to try this out to see what happens Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants