Error: Property is overwritten by later spread#27645
Error: Property is overwritten by later spread#27645
Conversation
Move the checking from JSX to getSpreadType so that normal objects get the error too. Also reword the missing semicolon related error to be less silly.
|
Note that the related span didn't work out because getSpreadType doesn't have easy access to the usage of properties, just their declarations. For spread properties, the declaration is a really bad site for a related span. I decided that the additional complexity of determining the usage site wasn't worth it. |
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS2710: 'children' are specified twice. The attribute named 'children' will be overwritten. | ||
| ~~~~~~~~~~~~~~~~~ | ||
| !!! error TS2735: 'children' is overwritten by a later spread. |
There was a problem hiding this comment.
Error message is inaccurate here
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS2710: 'children' are specified twice. The attribute named 'children' will be overwritten. | ||
| ~~~~~~~~~~~~~ | ||
| !!! error TS2735: 'children' is overwritten by a later spread. |
There was a problem hiding this comment.
..."later spread"? Methinks this needs a different error message, since it's not being overridden by a spread, but by the actual children tags in the AST.
| tests/cases/conformance/types/spread/objectSpreadNegative.ts(58,11): error TS2339: Property 'a' does not exist on type '{}'. | ||
| tests/cases/conformance/types/spread/objectSpreadNegative.ts(62,14): error TS2698: Spread types may only be created from object types. | ||
| tests/cases/conformance/types/spread/objectSpreadNegative.ts(65,14): error TS2698: Spread types may only be created from object types. | ||
| tests/cases/conformance/types/spread/objectSpreadNegative.ts(27,20): error TS2735: 'b' is overwritten by a later spread. |
There was a problem hiding this comment.
I don't think it's safe to emit this error outside of strictNullChecks in normal circumstances (JSX children being one of the few where it is).
We should have a related span that calls out which following spread (just in case there are multiple) is the one which overrides the property. |
|
@Andy-MS Actually, the related example that you gave in that issue also fails, with Basically, this fix is more complicated than I hoped, so I’m going to put it on hold until I address more urgent 3.2work. |
Doesn't fix JSX problems
|
3.2 has been out for a while -- any updates on this fix, or one that supersedes it? :D if there were complications, can they be described in a comment so we can understand better? |
RyanCavanaugh
modified the milestones:
TypeScript 3.6.1,
TypeScript 3.7.0,
TypeScript 3.7.1
sandersn
added
For Backlog Bug
|
#36727 is the revived version of this, which is now merged. |
6 participants
Move the checking from JSX to getSpreadType so that normal objects get the error too.
Also reword the missing semicolon related error to be less silly.
Fixes #27355 (probably)