Set symbol on union that is returned from getSpreadType
#18965
Conversation
Previously, it was only set on the top-level type, and only if that top-level type was an object type. Now it uses `forEachType` to set the symbol on every object type in the union as well, if `getSpreadType` returns a union.
Also tighten up the existing test code in the file.
src/compiler/checker.ts
Outdated
| t.flags |= propagatedFlags; | ||
| t.flags |= TypeFlags.FreshLiteral; | ||
| (t as ObjectType).objectFlags |= ObjectFlags.ObjectLiteral; | ||
| t.symbol = node.symbol; |
There was a problem hiding this comment.
Won't this trample over the existing symbol on any object types spread (should one be present)?
There was a problem hiding this comment.
Can we instead pass this symbol to getSpreadType and avoid the mutation after creating the type..
There was a problem hiding this comment.
@weswigham In its current state getSpreadType + the emptyObjectType check above will never trample an existing type. However, @mhegazy's idea is much less error-prone for future changes. Right now, getSpreadType is only called from places that have a symbol on hand anyway, since it's not a real type operator like it was planned to be.
There was a problem hiding this comment.
Done. It caused some churn in the ad-hoc JSX code, but it's probably worth it for the readability in checkObjectLiteral.
Previously, getSpreadType didn't set any flags and relied on its callers to do so. This was error-prone because getSpreadType often returns non-fresh types.
Fixes #16694
Previously, it was only set on a top-level type resulting from
getSpreadType, and only if that top-level type was an object type. NowcheckObjectLiteralusesforEachTypeto set the symbol on every object type in the union as well, ifgetSpreadTypereturns a union.