JSX errors use JSX terminology

[sandersn]

Well, as well as I understand it. Regardless, the error message no
longer mentions a complex synthetic type that represents the
component. It just mentions the name of the component.

Doesn't fix a specific bug, just makes everybody's life better.

Edit: The PR now simplifies the elaboration instead of the head message.

[RyanCavanaugh]

See what's going on here (unrelated)

[sandersn]

The second change in this test also shows off that we need to print entitynames, not just identifiers.

[sandersn]

Looks like IntrinsicAttributes is defined, but IntrinsicClassAttributes isn't. The predicate needs to give up a little less easily.

[weswigham]

IMO, an error like this is useless when it comes to actually fixing the problem if there's no elaboration. (Since then it doesn't mention where the issue is in the type, nor what the witnessed type actually is.) Which means if the same assignability error occurs twice in your program (potentially in very different places), and you happen to be looking at the elaboration-less site in your editor, fixing the problem could be quite the head-scratcher! While it may make the error less wordy, it removes a fair amount of utility, so I'm unsure about it.

[sandersn]

An error happening twice

1. Is rare (except when porting an existing program, but certainly when composing a new one).
2. Is not an issue in a real program since you'll be looking at the all the errors, and will presumably read them from top to bottom, so will see the elaborated version first.

The value of having an error message that JSX programmers can read far outweighs the problems with the unelaborated version. Specifically, the type literal that is synthesised from the component is foreign and confusing and I suspect that it makes new Typescript users stop reading.

[weswigham]

Is rare

Change a prop type used at multiple callsites (eg, a library or lib updates underneath you). Bam, multiple errors involving the same types.

Is not an issue in a real program since you'll be looking at the all the errors, and will presumably read them from top to bottom, so will see the elaborated version first.

90% of the time, I only look at the squiggles in the editor plus their messages and not the error output (this is mostly a workflow thing - I generally only use the error list to check for nonlocal errors); meaning I only immediately see errors around my current context unless I go looking for them.

Specifically, the type literal that is synthesised from the component is foreign and confusing and I suspect that it makes new Typescript users stop reading.

Just because the input looks like jsx instead of an object literal? I'm not sure I buy it. The shape's the same, it's just syntax that differs.

For comparison, the error

Type assigned to variable "x" which expects type "Foo" did not match.

Would be the analagous error for object literals, and I think that would be unambiguously worse, so why would the jsx error be any better?

Like, the tag name/variable name adds no more context than the error span itself (after all, the tag is readable there), so is extra info already provided elsewhere, so all you're doing is eliding half of the error and consequentially making it impossible to distinguish just from the error what went wrong (am I missing a literal type I expected? Is something an array when it shouldn't be?)

[sandersn]

@weswigham Let's talk about it in person with @RyanCavanaugh.

[sandersn]

We had a long in-person discussion. I'll try to summarise. First, the things we agreed on (I think -- feel free to chime in):

1. The main place this error happens is in the case that the component is missing an attribute. The children type could be wrong, but this doesn't happen much.
2. The top-level error line is not often useful -- both now, and after this change. Before, it lists a source type when only the property names even matter, and after the type is gone entirely. Ideally, the top-level error would be a combination of the first two error lines: "Property 'P' is required by type 'Target' but was not provided." No elaboration.
3. This is a general problem that object literal assignment shares with JSX components.
4. Even more generally, nested object literals (and JSX components with attributes with nested literal types) should drill down, discarding outer error messages until the first mismatch is encountered.

(4) is hard to implement, and the discard-drill-down idea needs some design thought in addition. I will try to specify it more carefully in a later post.

What we did not agree on is whether this change is an improvement without an elaboration. However, this is a moot point. JSX components are treated just like object literals, and multiple identical components are distinct, so each error has an elaboration.

[weswigham]

(4) is hard to implement, and the discard-drill-down idea needs some design thought in addition. I will try to specify it more carefully in a later post.

I have a branch with an implementation. It's often terser, for sure, but as I said in person - without related spans to retain some of the original context, I'm not very comfortable pushing it.

[sandersn]

In the meantime, I simplified the elaboration to mention JSX explicitly, and returned the head message to its original state. We might be able to generalise this change if we decide to implement related spans.

[weswigham]

@sandersn Now that my deeper elaborations (with related spans!) are in, what do you think about this?

[RyanCavanaugh]

@sandersn let's either refresh or close this

[sandersn]

This is outdated now, so I'l close it.