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

Multiple inheritance in the native component is ignored with RnWinRT. #212

Open
yusuketa opened this issue Mar 25, 2022 · 1 comment
Open
Labels
enhancement New feature or request

Comments

@yusuketa
Copy link

A native component I'm trying to reuse for React Native migration has multiple inheritance. A simplified code is like the following.

public interface INative1
{
    void method1();
}
public interface INative2: public INative1
{
    void method2();
}
public class Native: public INative2 {}

With this setup, only method2() is projected to JavaScript, and method1() is unavailable. The following code is not traversing up the inheritance chain but only looking through the direct parents. ☹

react-native-winrt-main\rnwinrt\rnwinrt\react\strings\base.cpp

projected_object_instance::projected_object_instance(const winrt::IInspectable& instance) : m_instance(instance)
{
    auto iids = winrt::get_interfaces(m_instance); <<<< Getting direct parents.
    for (auto&& iid : iids)
    {
        if (auto iface = find_interface(iid))
        {
            m_interfaces.push_back(iface); <<<< Not pushing back the ancestors of the direct parent interface.
        }
    }
}
@dunhor
Copy link
Member

dunhor commented Mar 25, 2022

For a quick reference on behavior and functionality, interfaces have two important functions in WinRT that are relevant here: QueryInterface and GetIids. QueryInterface is how you get a pointer to a particular interface that a type supports, and GetIids is how a type "advertises" which interfaces it supports. In general, these two sets of interfaces - which Interfaces/IIDs QueryInterface supports and returns success/a valid pointer back for, as well as which IIDs are included in the list returned from GetIids - should be the same (there's an exception where interfaces can support an interface, but not advertise it; these are typically referred to as "cloaked" interfaces, but this is not really relevant to this discussion). WinRT does not have proper interface inheritance. Therefore, although your idl defines INative2 as deriving from INative1, it really doesn't and instead only derives from IInspectable. Therefore, in order to call functions on the INative1 interface, consuming code would need to call QueryInterface with INative1's IID, get back a pointer to the interface, and then call the function.

RnWinRT doesn't really deal with types in this way - the developer does not indicate which interface it wishes to invoke a particular function on. Instead, it relies on the GetIids function to get a list of supported interfaces on a per-object basis. Therefore, if you are seeing that you are able to invoke functions on INative1 in non-JavaScript code (i.e. the code calls QueryInterface, which succeeds), but are unable to invoke the same functions on the same objects using RnWinRT (i.e. GetIids does not advertise the interface), then there's a bug in your component. The type's QueryInterface method responds to both INative1 as well as INative2, whereas its GetIids only advertises INative2.

That all said, interface requirements (which may "appear" to be like inheritance in idl definitions) do persist into the metadata (and I'm now slightly curious how Chakra handled this situation). So, in theory, we could modify each interface definition to have an additional array of required interfaces and then merge those into the m_interfaces array when constructing an object instance. I'm not a fan of that, however. This means more data, which means more binary bloat, and more runtime overhead to either de-duplicate interfaces on construction of an object interface, or to just add them all and then redundantly enumerate the same interfaces when looking for functions/properties. It seems much simpler and much more desirable for types to just behave as they are supposed to and advertise all interfaces that they support.

@dunhor dunhor added the enhancement New feature or request label Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants