Skip to content

Default implementation for IV8FastHostObjectOperations#683

Closed
AnsisMalins wants to merge 1 commit intoClearFoundry:masterfrom
decentraland:ioperations-default-impl
Closed

Default implementation for IV8FastHostObjectOperations#683
AnsisMalins wants to merge 1 commit intoClearFoundry:masterfrom
decentraland:ioperations-default-impl

Conversation

@AnsisMalins
Copy link
Contributor

This change adds a default implementation to IV8FastHostObjectOperations on runtimes that support it. The use case is objects exported to JavaScript that are part of an inheritance hierarchy. Without the default implementation, one has to laboriously implement every interface method every time.

I find it unlikely you will just merge this, but I'm hoping for some insightful feedback.

Usage:

class One { }

class Two : One, IV8FastHostObject, IV8FastHostObjectOperations
{
    void Bing() { }
    
    IV8FastHostObjectOperations IV8FastHostObject.Operations => this;

    private static readonly V8FastHostMethodInvoker<Two> BING =
        static (Two self, in V8FastArgs args, in V8FastResult result) =>
            self.Bing();
    
    void IV8FastHostObjectOperations.GetProperty(IV8FastHostObject instance, string name, in V8FastResult value, out bool isCacheable) =>
        GetProperty(name, value, out isCacheable);

    protected virtual void GetProperty(string name, V8FastResult value, out bool isCacheable)
    {
        isCacheable = true;

        switch (name)
        {
            case nameof(Bing):
                value.Set(BING);
                break;
            default:
                throw new NotImplementedException(
                    $"Named property {name} is not implemented");
        }
    }
}

class Three : Two
{
    void Bong() { }
    
    private static readonly V8FastHostMethodInvoker<Three> BONG =
        static (Three self, in V8FastArgs args, in V8FastResult result) =>
            self.Bong();
    
    protected override void GetProperty(string name, V8FastResult value, out bool isCacheable)
    {
        isCacheable = true;

        switch (name)
        {
            case nameof(Bong):
                value.Set(BONG);
                break;
            default:
                base.GetProperty(name, value, out isCacheable);
                break;
        }
    }
}

@AnsisMalins AnsisMalins marked this pull request as draft October 14, 2025 11:33
@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Oct 15, 2025

Hi @AnsisMalins,

Thanks for posting this PR!

I find it unlikely you will just merge this, but I'm hoping for some insightful feedback.

Here's why we're reluctant to merge this PR as is:

  • According to the C# Language Reference, the motivation for default interface methods is the ability to expand existing interfaces without introducing breaking changes. In other words, the feature's primary purpose is not to make interfaces easier to implement, but rather to make them easier to evolve.
  • IDEs such as Visual Studio can generate interface implementations that throw NotImplementedException in one step.
  • On the other hand, GetFriendlyName should be easier to implement. We feel, however, that the right way to do that would be to provide a public API rather than a default method in one specific interface.

Thanks again!

@AnsisMalins
Copy link
Contributor Author

What's your prescription for implementing FastProxy compatibility for types who's "inheritance slot" is already taken? Just implement the interface?

@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Oct 16, 2025

What's your prescription for implementing FastProxy compatibility for types who's "inheritance slot" is already taken? Just implement the interface?

You could implement a "fast wrapper" for your class using V8FastHostObject, or you could add an IV8FastHostObject implementation to the class.

Here's a simple superclass-subclass hierarchy in which both classes have FastProxy support:

public class Foo : IV8FastHostObject {
    public long Value { get; set; }
    public double GetScaledValue(double scaleFactor) => Value * scaleFactor;
    // FastProxy stuff
    private static readonly V8FastHostObjectOperations<Foo> Operations = new();
    IV8FastHostObjectOperations IV8FastHostObject.Operations => Operations;
    protected static void Configure<T>(V8FastHostObjectConfiguration<T> configuration) where T : Foo {
        configuration.AddPropertyAccessors(
            nameof(Value),
            static (T self, in V8FastResult value) => value.Set(self.Value),
            static (T self, in V8FastArg value) => self.Value = value.GetInt64()
        );
        configuration.AddMethodGetter(
            nameof(GetScaledValue),
            static (T self, in V8FastArgs args, in V8FastResult result) => result.Set(self.GetScaledValue(args.GetDouble(0)))
        );
    }
    static Foo() => Operations.Configure(static configuration => Configure(configuration));
}
public class Bar : Foo, IV8FastHostObject {
    public double GetReducedValue() => Value / (2.0 * Math.PI);
    // FastProxy stuff
    private static readonly V8FastHostObjectOperations<Bar> Operations = new();
    IV8FastHostObjectOperations IV8FastHostObject.Operations => Operations;
    protected new static void Configure<T>(V8FastHostObjectConfiguration<T> configuration) where T : Bar {
        Foo.Configure(configuration);
        configuration.AddMethodGetter(
            nameof(GetReducedValue),
            static (T self, in V8FastArgs _, in V8FastResult result) => result.Set(self.GetReducedValue())
        );
    }
    static Bar() => Operations.Configure(static configuration => Configure(configuration));
}

With this setup, the subclass effectively inherits and extends its FastProxy configuration. Beyond the class-specific configuration code, FastProxy support amounts to just a few lines of boilerplate.

@ClearScriptLib ClearScriptLib self-assigned this Oct 16, 2025
@AnsisMalins AnsisMalins deleted the ioperations-default-impl branch October 23, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants