-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Support serialization of CLR Classes #2025
Support serialization of CLR Classes #2025
Conversation
…ted) Replaced CreateInstanceInternal from ModelBindingExtensions.cs with .net framework Activator.CreateInstance that does the job better
In what context? Assuming you mean allow
What does this mean exactly? |
@@ -36,7 +36,7 @@ public static T CreateInstance<T>(this Type type, bool nonPublic = false) | |||
/// <param name="nonPublic"><see langword="true"/> if a non-public constructor can be used, otherwise <see langword="false"/>.</param> | |||
public static object CreateInstance(this Type type, bool nonPublic = false) | |||
{ | |||
return CreateInstanceInternal(type, nonPublic); | |||
return Activator.CreateInstance(type, nonPublic); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this change? How does it relate to structs
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Structs have implicit public parameterless constructors and the CreateInstanceInternal wasn't able to find that constructor. So I replaced with the Activator.CreateInstance which does the job right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much clearer, thank you.
Please provide an example that demos your changes. |
Will look at merging this soon, we need to add support for |
Can't the properties support be made in another pull request and merge this one? |
I'd prefer for this to be complete. What's the advantage of merging this now? |
I would like to move to cefsharp master other than maintaining my own branch with fixes. But I don't have bandwidth to do those changes now. |
@joaompneves This one has gone a bit stale. Would you have a chance to update it to resolve the conflicts? |
❌ Build CefSharp 63.0.0-CI2403 failed (commit e5b261aa97 by @joaompneves) |
✅ Build CefSharp 63.0.0-CI2404 completed (commit 93c572d863 by @joaompneves) |
Not working with recent changes |
@amaitland Do you think this is superseded by the recent JSB changes you did? I am thinking about 7a0b06a. (Note: haven't looked at the PR myself.) |
Support for returning a Returning properties and fields for both structs and classes is now possible ( |
Bound objects (using RegisterAsyncJsObject) now can have methods that return Structs and Classes (before only structs were allowed). This PR introduces support for classes (returned to the JS side) serialization (only public fields will be serialized). Check the Binding examples: asyncStructsArray and asyncClassesArray.