Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dotnet): AnonymousObject fails runtime type checks #3709

Merged
merged 7 commits into from
Aug 18, 2022

Conversation

RomainMuller
Copy link
Contributor

The AnonymousObject class is used when a value is passed from JS to
.NET through a union or any typed return point, and there is not
sufficient runtime type information on the value to decisively identify
its dynamic type. The AnonymousObject class will be converted by the
jsii runtime to any interface type implicitly (even if that is
technically not correct), and it must hence be allowed through runtime
type-checks for interfaces. Essentially, this is a blind spot of the
runtime type checks, and should not cause valid code to break.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

The `AnonymousObject` class is used when a value is passed from JS to
.NET through a union or `any` typed return point, and there is not
sufficient runtime type information on the value to decisively identify
its dynamic type. The `AnonymousObject` class will be converted by the
jsii runtime to any interface type implicitly (even if that is
technically not correct), and it must hence be allowed through runtime
type-checks for interfaces. Essentially, this is a blind spot of the
runtime type checks, and should not cause valid code to break.
@RomainMuller RomainMuller added bug This issue is a bug. language/dotnet Related to .NET bindings (C#, F#, ...) p1 labels Aug 17, 2022
@RomainMuller RomainMuller requested a review from a team August 17, 2022 07:56
@RomainMuller RomainMuller self-assigned this Aug 17, 2022
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Aug 17, 2022
Comment on lines +723 to +726
const varName = `__idx_${createHash('sha256')
.update(descr)
.digest('hex')
.slice(0, 6)}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated, but makes the generated code more readable, which makes it less likely that a bad change is missed by human scrutiny.

Comment on lines +747 to +750
const varName = `__item_${createHash('sha256')
.update(descr)
.digest('hex')
.slice(0, 6)}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated, but makes the generated code more readable, which makes it less likely that a bad change is missed by human scrutiny.

@RomainMuller RomainMuller changed the title fix(dotnet): AnonymousObject faisl runtime type checks fix(dotnet): AnonymousObject fails runtime type checks Aug 17, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 17, 2022

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Aug 17, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 17, 2022

Merging (with squash)...

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Aug 17, 2022

Merging (with squash)...

@mergify
Copy link
Contributor

mergify bot commented Aug 17, 2022

Merging (with squash)...

@RomainMuller
Copy link
Contributor Author

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Aug 18, 2022

refresh

✅ Pull request refreshed

@mergify mergify bot merged commit e7fadc0 into main Aug 18, 2022
@mergify mergify bot deleted the rmuller/dotnet-anonymous-type-check branch August 18, 2022 08:23
@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. contribution/core This is a PR that came from AWS. language/dotnet Related to .NET bindings (C#, F#, ...) p1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants