Skip to content

Conversation

bentongxyz
Copy link
Contributor

Fixes #49235

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Jun 1, 2022
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I think this is correct, although I'd like to get a second opinion from @gabritto. I have a couple of suggestions too.

&& !(type.symbol.flags & SymbolFlags.Class)
&& !typeHasCallOrConstructSignatures(type)
) || !!(
objectFlags
Copy link
Member

Choose a reason for hiding this comment

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

getObjectFlags always returns a value, so you don't need to check for undefined (or 0)

) || !!(
objectFlags
&& objectFlags & ObjectFlags.ObjectRestType
) || !!(getObjectFlags(type) & ObjectFlags.ReverseMapped && isObjectTypeWithInferableIndex((type as ReverseMappedType).source));
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace getObjectFlags(type) with objectFlags here too? getObjectFlags is likely to be inlined, so you could also call it from each place.

@sandersn sandersn requested a review from gabritto June 9, 2022 22:16
@sandersn sandersn self-assigned this Jun 9, 2022
@bentongxyz
Copy link
Contributor Author

@sandersn I have made updates accordingly, thank you for the review :)

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I'll leave for a few days to see what @gabritto thinks.

@gabritto gabritto merged commit dbab6eb into microsoft:main Jun 14, 2022
@bentongxyz bentongxyz deleted the 49235 branch June 15, 2022 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Backlog Bug PRs that fix a backlog bug

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[Bug] Result of object rest isn't assignable to symbol index signature

4 participants