-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Provide flattened.error.reporting(...).for(...).nested.incompatible.types #33361
Comments
I explored this a bit with @alloy when thinking about whether we could also use bold/italics to also provide emphasis, instead we used I prefer the version in OP. I wonder if turning it into three lines would make it more readable:
Then you get to see the full message without scrolling, and can more easily compare the types. Ideally, if we're in an environment where we can use bold:
Which would really help lead someone's eyes to the right spots to make their own decisions. |
I'd like to try this. |
@dhruvrajvanshi Go for it! It might be a little bit tricky because this is deep in the logic for To scope the problem space, I'll call the special diagnostic
a member chain diagnostic. A member chain diagnostic is created by tracking checks between properties and call signatures, and determining if multiple relevant properties/call signatures are encountered consecutively. A member is relevant for these checks if the member is
I like that @orta. Maybe that can be layered on top for the first message, even if we can't do styling on diagnostics yet. I'd open up a separate issue if you're interested! |
(Please forgive me for my poorly written spec 😅) |
By the way, we'd like to get this in for the beta if possible. Do you think you can get this done by the end of the month @dhruvrajvanshi? |
@DanielRosenwasser In that case, you might want to assign this to someone else. I can't really commit to a deadline right now. |
@DanielRosenwasser I think I could commit to getting this done by then. Is there someone/somewhere in particular I should ask for guidance on this? Or just here on the issue? |
I think this issue is a good place to leave notes and questions 👍 |
Just wanted to give a small update and see if I'm on the right approach. So far I've mostly just been getting an understanding of how
Could this be done by keeping a linked list structure (or even just an array) defined in Assuming that is correct, and once we have a "member chain" built, before Hopefully I'm on the right track, and I may open a draft PR if that is ok to sort of show the work being done. |
@austincummings sorry my friend, but as of earlier today, we have a pretty much complete implementation that we should have a PR up for by EoD tomorrow - the assignment was supposed to indicate that~ Hopefully it'll at least be educational? 🤷♀ |
@weswigham no worries. I'm sure it will be. Thanks for letting me know. |
May I suggest a small whitespace tweak to make the new and awesome error a bit more readable?
move the
or, to minimize horizontal scrolling/overflow place it in a line in between the types:
Happy to make such a PR :) |
Individual multiline diagnostic messages I think require a couple changes to our diagnostic message chain builder, but I'd be down for considering it, personally. It's worth remembering that it could still occur in a pyramid, so you could have something like
which is.... maybe better? I dunno, linebreaks, man. I know I probably don't wanna see
so there'd probably need to be some kind of length heuristic, at least. |
Brainstorming about the (currently) impossible here, I would say that the thing that Would the TypeScript team be open to a pipe dream issue about this for some future? |
So while that's pretty ok in the extreme example from the op, most[citation needed] types involved in larger errors aren't deeply nested anonymous objects, but are instead interfaces within interfaces - so your top level type error is usually something like |
Ok, then I'll avoid creating a new issue for it. Thanks for the feedback! |
It would be wonderful to have more approachable error messages in situations like this: Here, the problem is related to index signatures. The expected type has one, the provided type doesn't. In order to see what the real problem is, one needs to read a pretty big and intimidating error message in which the important message is on the bottom. |
@karol-majewski "just print the error messages in reverse order" is a common suggestion; see #29759 (comment) . TL;DR it makes some things better and other things worse |
@RyanCavanaugh Thank you for your reply. Reversing the order is one dimension and I agree this is not a perfect solution everywhere. What I had in mind was making the error message say which object is missing the index signature. Right now we only know one of them is missing it.
My hope is this could point the users into the right direction: finding a way to update the object in a way that preserves its index signature (which the spread operator |
Existing Behavior
Proposed
The text was updated successfully, but these errors were encountered: