vs-streamjsonrpc
vs-streamjsonrpc copied to clipboard
microsoft
Allow `[RpcMarshalable]` interfaces to declare properties with serialized values
Some objects that are marshaled via their [RpcMarshalable] interface would like to include properties on that interface. Since all RPC members must be async and properties are not async, these properties would not invoke RPC. Rather, they would return values that were captured from the marshaled object at the time it was originally marshaled. Essentially, sending an object via a marshaled interface may include two parts: a serialized form of its declared properties, and the marshaled handle.
This could create a usability problem when receivers get an interface where the methods invoke RPC but properties return stale values from when they first received the object. We can mitigate this by making this opt-in on a per-interface basis. An interface with [RpcMarshalable, RpcSnapshotProperties] would make properties allowable, and also effectively communicate to the interface author and its users that the properties will retain only snapshotted values.
@matteo-prosperi What do you think? Feedback? Any interest in implementing this?
I would not add a second attribute: it seems too verbose to me. I don't have a good reason for rejecting this, except that I like the current clean separation between POCOs and marshalable objects. Currently, if I remember correctly, marshalable interfaces cannot have properties, so this would not introduce any backward incompatibility. It's worth thinking about polymorphism since marshalable interfaces now support it and interface properties can also be polymorphic.
On Wed, Jul 26, 2023, 16:46 Andrew Arnott @.***> wrote:
@matteo-prosperi https://github.com/matteo-prosperi What do you think? Feedback? Any interest in implementing this?
— Reply to this email directly, view it on GitHub https://github.com/microsoft/vs-streamjsonrpc/issues/952#issuecomment-1651959872 or unsubscribe https://github.com/notifications/unsubscribe-auth/AKAGVXVINBRP6VI7D3Z7LKDXSEUUBBFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLLDTOVRGUZLDORPXI6LQMWWES43TOVSUG33NNVSW45FGORXXA2LDOOJIFJDUPFYGLKTSMVYG643JORXXE6NFOZQWY5LFVA3TAMBWHAZDGMUCUR2HS4DFUVUXG43VMWSXMYLMOVS2UMJYGIZDKOBSGU2TRJ3UOJUWOZ3FOKTGG4TFMF2GK . You are receiving this email because you were mentioned.
Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .
I suppose rather than a new attribute we could add a property to the existing one. I like the clean separation as well, but it has led to some awkward API changes being required in the VS copilot extension.
I think having a property in the attribute is a good idea.
On Wed, Jul 26, 2023, 22:29 Andrew Arnott @.***> wrote:
I suppose rather than a new attribute we could add a property to the existing one. I like the clean separation as well, but it has led to some awkward API changes being required in the VS copilot extension.
— Reply to this email directly, view it on GitHub https://github.com/microsoft/vs-streamjsonrpc/issues/952#issuecomment-1652449144 or unsubscribe https://github.com/notifications/unsubscribe-auth/AKAGVXSWKUVUQ7K43VNV5VDXSF43LBFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLLDTOVRGUZLDORPXI6LQMWWES43TOVSUG33NNVSW45FGORXXA2LDOOJIFJDUPFYGLKTSMVYG643JORXXE6NFOZQWY5LFVA3TAMBWHAZDGMUCUR2HS4DFUVUXG43VMWSXMYLMOVS2UMJYGIZDKOBSGU2TRJ3UOJUWOZ3FOKTGG4TFMF2GK . You are receiving this email because you commented on the thread.
Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .