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

Hide Call method from public API #1062

Merged
merged 2 commits into from Apr 24, 2022
Merged

Conversation

lahma
Copy link
Collaborator

@lahma lahma commented Jan 22, 2022

Hiding Call like we hid Construct, it's now protected internal and ICallable is explicit implementation, but can still be accessed via extension method helper that gets the Engine instance and then invokes the call via it.

Engine.Call allows to get via name, use existing JsValue and pass parameters, overload to also supply thisObj that requires using proper array as arguments parameter to disambiguate and follow JS style.

@lahma lahma force-pushed the hide-callable branch 5 times, most recently from 7eaa498 to db5f529 Compare January 22, 2022 17:00
@lahma lahma marked this pull request as ready for review January 22, 2022 17:11
@CreepGin
Copy link
Contributor

CreepGin commented Jan 23, 2022

Just tested this on my side. Everything seems to work well. But the inability to set thisObj breaks my old code that depends on it. Here's one of my use cases:

I'm interop'ing C# with Preact (basically a light weight React). So all the dom APIs that Preact depends on, I implemented them in C#.

// Preact code
function eventProxy(e) {
	this._listeners[e.type + false](options.event ? options.event(e) : e);
}
dom.addEventListener(name, eventProxy, useCapture);
// Example dom api that I have to implement. I need to pass the dom obj back as 'this' for eventProxy(e)
public class dom {
    public Dictionary<string, object> _listeners;

    ...

    public void addEventListener(string name, JsValue jsval, bool useCapture = false) {
        var func = jsval.As<FunctionInstance>();
        var engineThis = JsValue.FromObject(UIEngine.Instance.Engine, this);
        var callback = (EventCallback<EventBase>)((e) => {
            func.Call(engineThis, JsValue.FromObject(UIEngine.Instance.Engine, e));  // <-- passing dom as 'this'
        });
        switch (name) {
            case "MouseUp":
                _ve.RegisterCallback<MouseUpEvent>(callback);
                break;
            ...
        }
        _registeredCallbacks.Add(name, callback);
    }
}

@lahma
Copy link
Collaborator Author

lahma commented Jan 23, 2022

@CreepGin thanks for the use case, I've updated the PR to include overloads to allow passing in the wanted thisObject.

@CreepGin
Copy link
Contributor

Awesome! Looks and works well on my side.

@lahma lahma enabled auto-merge (squash) April 24, 2022 16:13
@lahma lahma merged commit bbc5bab into sebastienros:main Apr 24, 2022
@lahma lahma deleted the hide-callable branch April 24, 2022 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants