Skip to content

Conversation

@jakebailey
Copy link
Member

#51580 had not been updated in a while and has some baseline changes.

I am not sure the behavior of intraExpressionInferencesJsx is desirable here. @Andarist

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 16, 2023
@jakebailey jakebailey mentioned this pull request Mar 16, 2023
@weswigham
Copy link
Member

It's definitely not desirable - I've been looking at it and it seems like #51580 and #52837 conflict semantically, to the tune of partially re-breaking what #52837 fixed. (Also the null thing, but that's just a change we made recently.)

In the meantime, we can revert #51580 to put main right again, imo.

@weswigham
Copy link
Member

weswigham commented Mar 16, 2023

(Specifically, by no longer passing in the checkMode, we're missing the intra-expression sites in the nested expression. We should probably be specifying the checkMode, but be skipping the error reporting when checkMode isn't Normal, and I did a quick test of that - diff is here - but I haven't had time to check if all those baseline changes are OK; they may just be highlighting even more scenarios where CheckMode isn't being propagated where it should be, ungh)

@jakebailey
Copy link
Member Author

Sent #53283.

@jakebailey jakebailey closed this Mar 16, 2023
@jakebailey jakebailey deleted the fix-baselines branch March 16, 2023 02:15
@jakebailey jakebailey mentioned this pull request Mar 16, 2023
Comment on lines +109 to +114
<Foo {...{
a: (x) => 10,
b: (arg) => { arg.toString(); },
~~~
!!! error TS18046: 'arg' is of type 'unknown'.
}} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I see this is the only "unwanted" change that has been found on main. However... this is exactly how regular call expressions work: TS playground. So perhaps... this was just working accidentally before. I feel like this should be a separate issue to make it work for both JSX and regular calls (if that's desired)

@Andarist
Copy link
Contributor

We should probably be specifying the checkMode, but be skipping the error reporting when checkMode isn't Normal, and I did a quick test of that - diff is here - but I haven't had time to check if all those baseline changes are OK; they may just be highlighting even more scenarios where CheckMode isn't being propagated where it should be, ungh)

I took a look at those changes and I think that some of them are not desired.

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

4 participants