Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Custom marshal attribute #25

Closed
wants to merge 1 commit into from
Closed

Custom marshal attribute #25

wants to merge 1 commit into from

Conversation

jasongin
Copy link
Owner

@jasongin jasongin commented Feb 18, 2023

This is a proposal for a custom marshalling attribute and interface. Custom marshalling is not actually implemented yet in the code-generator. The design is mostly modeled after System.Runtime.InteropServices.MarshalAsAttribute.

/// <summary>
/// Specifies how .NET types are marshalled to and from JavaScript values.
/// </summary>
[AttributeUsage(
Copy link
Collaborator

Choose a reason for hiding this comment

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

AttributeUsage

Should we also add fields?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Later, if and when we support marshalling fields.

Fields are not supported now, but there's no reason they can't be marshalled in almost the same way as properties, along with an optimization for const fields. I don't think it's a priority now though.

/// where the type argument matches the type being of the property, parameter, or
/// return value the <see cref="JSMarshalAsAttribute" /> is applied to.
/// </summary>
public Type? CustomMarshalType { get; init; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Type?

I wonder if we can use the new generic attributes instead. In that case the restriction that the type must inherit from IJSMarshaler can be written as an explicit restriction checked by the C# compiler.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I thought about that, but we'd still need a non-generic JSMarshalAsAttribute to support uses that don't specify a custom marshaller type. So I didn't think it was worth creating another attribute.

Copy link
Collaborator

@vmoroz vmoroz left a comment

Choose a reason for hiding this comment

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

:shipit:

@jasongin
Copy link
Owner Author

I'm abandoning this PR because it's not a top priority at the moment, and was mainly meant as a starting point for discussion. I do still plan to implement this idea eventually. Since opening this PR I've done a lot more work related to marshalling, so I think there can be some improvements to this design when I bring it back.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants