Currently we only have a single class with a default constructor. We should also test cases where a class has multiple constructors. I'll also throw in a placeholder here to check if constructor "overloading" with the same arity is supported. I believe I put in an assert here, so if it is supported, there might be a small bug fix.

I believe we kind of test this already, but not in depth. We should have a scenario where we're testing getting an object as two different interfaces and validate that they're the same. E.g. we may have a type Foo with a default interface IFoo that also implements some other (non-exclusiveto) interface IBar and have a function that takes in a Foo and returns the object QI'd as an IBar. These two should test equal in JS

Currently most of our tests are off of a single class that has both static methods/properties/events and constructors. Given the unfortunate reality that there's a difference between the code-behind for activatable and non-activatable types, we should have at least some number of tests for statics-only types.

There's a number of places where we need to declare an "empty" type - e.g. arrays, out params, etc. - and with C++/WinRT, if we declare a variable of class type with no initializer, this results in the type getting activated with no arguments (i.e. instead of constructing a null instance on the stack/wherever). We should add scenarios where we use non-default activatable classes in these types of situations. This can probably be done in tandem with #61

I have tried testing to see if JsWinRT would work with RNW 0.64

Compilation Issues
(1) react-native-patched no longer exists so include paths in WinRTTurboModule.vcxproj so include paths should point to various folders under react-native instead of $(ReactNativeWindowsDir)build\$(ReactPlatform)\$(Configuration)\ReactCommon\react-native-patched.

Given <ReactCommonDir>$(ReactNativeWindowsDir)..\react-native\ReactCommon\</ReactCommonDir>

  <AdditionalIncludeDirectories>
    $(ReactCommonDir)jsi;
    $(ReactCommonDir)callinvoker;
    $(ReactCommonDir)react\nativemodule\core;
    $(GeneratedFilesDir);
    %(AdditionalIncludeDirectories);
  </AdditionalIncludeDirectories>

(2) base.h refers to TurboModule/Core/TurboModule.h but this should now just be ReactCommon\TurboModule.h assuming the include paths in (1)

(3) dllmain.cpp refers to TurboModule/Core/TurboModule.cpp but this should now be ReactCommon\TurboModule.cpp assuming the include paths in (1)

(4) The patch for TurboModuleManager.cpp should change slightly as turbomodule/samples/SampleTurboCxxModule.h changed to ReactCommon/SampleTurboCxxModule.h but per vmoroz's comment, we should really be using AddTurboModuleProvider to expose the WinRTTurboModule with a ReactPackagedProvider which supports auto-linking too. To accomplish a WinRTTurboModule.vcxproj should add dependency on Microsoft.ReactNative.Uwp.CppLib.props+targets, remove jsi.cpp and turbomodule.cpp from dllmain.cpp and WinRTTurboModule.def should export DllMain, DllCanUnloadNow, and DllGetActivationFactory, and define a ReactPackageProvider like a normal Native Module. A WinRTTurboModule.h should be added that forward declares WinRTTurboModule so it can be used with AddTurboModuleProvider and CreateWinRTTurboModule should be deleted along with patching of TurboModuleManager.cpp. It would also be advisable to a WinRTTurboModule.vcxproj.filters to hide files added by Microsoft.ReactNative.Cxx.vcxitems. Some additional fixes like removing CppWinRTGenerateWindowsMetadata=false are likely necessary.

(5) Other minor issues that will need to be addressed for the sample project like changing metro.config.js's reference to blackList to blockList to be something like the following:

const exclusionList = require('metro-config/src/defaults/exclusionList');
module.exports = {
  resolver: {
    blocklist: exclusionList([
      new RegExp(
        `${path.resolve(__dirname, 'windows').replace(/[/\\]/g, '/')}.*`,
      ),
      /.*\.ProjectImports\.zip/,
    ]),
  },

Functional Issues

1. Constructor calls appear to fail with Constructor call for HostObjectGetTrap() is not supported.. This appears to be a regression due to ChakraRuntime.cpp changes in microsoft/react-native-windows#5710 that added exceptions if trying to retrieve a constructor from the HostObject. Thankfully, the naïve fix of commenting out the two throw-if statements (in ChakraRuntime::HostObjectGetTrap and ChakraRuntime::HostObjectSetTrap) fixes the issue (and I can see the telemetry logged with Windows.Foundation.Diagnostics.LoggingChannel())

 	Microsoft.ReactNative.dll!Microsoft::JSI::ChakraRuntime::ThrowNativeExceptionOverride(const char * errorMessage) Line 589	C++
>	Microsoft.ReactNative.dll!Microsoft::JSI::ChakraApi::ThrowNativeException(const char * errorMessage) Line 83	C++
 	Microsoft.ReactNative.dll!Microsoft::JSI::ChakraRuntime::HostFunctionCall::__l2::<lambda>() Line 678	C++
 	Microsoft.ReactNative.dll!Microsoft::JSI::ChakraRuntime::HandleCallbackExceptions<void * <lambda>(void)>(Microsoft::JSI::ChakraRuntime::HostFunctionCall::__l2::void * <lambda>(void) lambda) Line 297	C++
 	Microsoft.ReactNative.dll!Microsoft::JSI::ChakraRuntime::HostFunctionCall(void * __formal, bool isConstructCall, void * * args, unsigned short argCount, void * callbackState) Line 689	C++
 	Chakra.dll!
 	...
 	Microsoft.ReactNative.dll!Microsoft::JSI::ChakraApi::CallFunction(void * function, Microsoft::JSI::ChakraApi::Span<void *> args) Line 416	C++
 	Microsoft.ReactNative.dll!Microsoft::JSI::ChakraRuntime::call(const facebook::jsi::Function & func, const facebook::jsi::Value & jsThis, const facebook::jsi::Value * args, unsigned __int64 count) Line 540	C++
 	Microsoft.ReactNative.dll!facebook::jsi::Function::call(facebook::jsi::Runtime & runtime, const facebook::jsi::Value * args, unsigned __int64 count) Line 228	C++
 	Microsoft.ReactNative.dll!facebook::jsi::Function::call(facebook::jsi::Runtime & runtime, std::initializer_list<facebook::jsi::Value> args) Line 233	C++
 	Microsoft.ReactNative.dll!facebook::jsi::Function::call<std::string const &,std::string const &,facebook::jsi::Value>(facebook::jsi::Runtime & runtime, const std::string & <args_0>, const std::string & <args_1>, facebook::jsi::Value && <args_2>) Line 241	C++
 	Microsoft.ReactNative.dll!facebook::react::JSIExecutor::callFunction::__l6::<lambda>() Line 231	C++
 	...

<I will add on to this list if I discover more during testing, but I am as of yet blocked on the first issue discovered..>

"react": "17.0.1",
"react-native": "0.64.0-rc.3",
"react-native-windows": "0.64.0-preview.12"

E.g. a property getter on IFoo and the setter added to IFoo2

When I originally added DebugBundle to my prototype projects early last year I was working with an old version of RNW. I don't recall seeing an option to use "react-native start" and SourceBundleHost + SourceBundlePort but with the built-in JS engine but might have missed it. So I came up with the idea of using DebugBundle and didn't look further until now.

It turns out that the UWP app connecting to the dev server with fast refresh while using the in-proc JS engine with working is actually possible either by setting UseWebDebugger=false (and optionally UseDirectDebugger=true) or by using the Ctrl+Shift+D menu and clicking "Disable Remote JS Debugging". It will then reload and projections should work if they had been working with DebugBundle/Release before.

We somewhat already test IPropertyValue, but this is for a single property and isn't really in-depth. We should do our best to exercise as much of convert_to_property_value and convert_from_property_value as possible. We should also do this for both scenarios that explicitly mention IPropertyValue as well as IInspectable, since we treat both as possibly property value-like.

Not really a test, but more of validation. It's a bit annoying to have to temporarily change other vcxproj files just to test building the SDK. We should have a separate project just for this. Should probably also include the -webhosthiddenincluded flag just to expand test coverage.

Not sure if we'd be able to run this as part of the CI build (ideally we would) because it might consume too many resources. I doubt it since I previously was able to get pretty far with building all of LLVM - and we are much smaller - but that's something to note.

Due to the way C++/WinRT handles class type constructors (it turns into an activation), it's not okay to declare class types without having a nullptr initializer. Since we currently compile the entire SDK, we're probably fine on this front, however it's always good to not have to rely on doing that. In particular it would be great to have:

• Function output args and return values (incl. properties)
• Generic type arguments
• Delegate input/output args and return values

It should go without saying that the class used for all of these should not be default activatable.

Context

I'm working on react-native-xaml which allows projecting XAML types for use in RNW apps via the ViewManager API, and allows apps to use these XAML types directly in their JSX markup.

One of the features that is important to support is setting up event handlers from JS; these event handlers need to be able to get event args.
Currently the support in react-native-xaml for event args is fairly limited. Basically the way this works is we try to serialize what we consider important properties on every relevant type. For example, for every UIElement we will serialize the Name and Tag properties, for every object we will serialize the runtime class name, for MenuFlyoutItem we would serialize the Text properties, for SelectionChangedEventArgs we serialize what things were added and removed to the selection, etc.

Feature request

I'd like to eventually get out of the business of having to do this serialization manually, and instead rely on jswinrt for it.
Basically I'd like to have a native API from jswinrt that looks like:

void JSWinRT::Serialize(
    const winrt::Microsoft::ReactNative::JSValueWriter& writer, 
    const winrt::Windows::Foundation::IInspectable& obj)

Seems that the generated code uses value as the name for the output return value, which will conflict with the argument name.

Static functions and member properties (get/put) all work fine, however static properties don't seem to work. FWIW, there's also evidence in the sample that things like enum values - which behave similar to properties, just without the get/put calls - do work. I've debugged this a bit, but didn't get very far. I can see machinery attempting to set up support, however I saw no indication that it was working.

The IAsyncAction/Operation are theoretically accurate but dont match the actual projection since the projection special cases things a bit to match Chakra and takes a shortcut by not treating them like other projected interfaces so it excludes IAsyncInfo on the type itself and exposes the actual underlying operation as property operation. For example, setting the handlers for completed and progress handlers isn't possible as the Promise semantics are assumed instead by using a then(success, fail) instead with an additional progress param. There are also some additional semantics like a cancel method and a semi-obsolete done (included only for back-compat). May be worth taking a look at base.h and base.cpp.

We have several delegate tests. Would be good to include these as well. Might be interesting since they involve generic types.

There's a number of places where we voluntarily use std::function to hold callbacks (as opposed to e.g. JSI where usage is forced). This is sub-optimal from a binary size perspective since std::function prevents a lot of things from getting folded together. The root problem seems to be std::function's target_type and target functions. These require the object to hold RTTI info on the underlying function object - even though I've never seen or heard of anyone needing/wanting these functions - which then causes constructors & v-tables to need to be unique causing a whole host of functions to be non-foldable.

It would be relatively simple to substitute in our own std::function-like type that does not require this RTTI info, though it's not immediately clear if it would be worth it. Could be a good investigation, though.

This causes any AdditionalIncludeDirectories set in the vcxproj file or in other props/targets included before WinRTTurboModule.props to be discarded.

The namespaces aliases can causes collisions when attempting to include other headers so it would be safer to remove the aliases entirely or move them to the cpp files.

namespace jsi
{
    using namespace facebook::jsi;
}

namespace react
{
    using namespace facebook::react;
}

Trying to consume some windows sdk winmds crashes with following error:

Y:\xss2\packages\ExperienceExtensions\windows\WinRTTurboModule>Y:\xss2\packages\ExperienceExtensions\windows\packages\Microsoft.Windows.JsWinRT.20210209.2.0\rnwinrt\rnwinrt.exe -in "Y:\xss2\packages\ExperienceExtensions\windows\packages\Microsoft.Windows.SDK.cpp.10.0.21320.1000-preview.rs-prerelease\c\References\10.0.21320.0\Windows.Foundation.FoundationContract.winmd" -tsoutput "Generated Files\test"
error: element type not supported: FnPtr

The RnWinRTMakeProjections target generates C++ but now that there is support for generating TS declarations we should have it emitted too. Perhaps -tsoutput could be specified to some default in rnwinrt.rsp like it is for C++, or configurable with some props or a separate target.

The samples currently exercise this behavior, but none of the tests do. Specifically we should test:

• Overloaded functions of different arity on the same interface
• Overloaded functions of different arity on different interfaces
• Overloaded functions of the same arity on the same interface
• Overloaded functions of the same arity on different interfaces
  • Should include scenarios where the defaulted function is on either interface
  • Should include scenarios where multiple non-default functions exist on either interface
• Constructor overloads

This is to expand test coverage for #13. This is currently being tested with static events - activation factories typically don't support weak ref - however that code path is a fair bit different than object instances.

Fill arrays are somewhat special since they are represented in metadata as out, but not ref. This means that we have to treat them a bit specially which makes the logic a bit odd. We should cover the follow scenarios (some of which we already do):

• Fill array argument with no/void return and no output arguments
• Fill array argument with no/void return and one or more output arguments
• Fill array argument with non-void return and no output arguments
• Fill array argument with non-void return and one or more output arguments

The tool emits warnings when it encounters a delegate definition with an out param - saying out params currently are not supported - however if the delegate is actually used anywhere (e.g. as a function argument), the generated code gives compilation errors.

The original intent of the FAIL_FAST was likely to prevent instances where registered events are not unregistered before the underlying WinRT object is freed, given that ProjectedEventRegistrar only holds a weak ref (#13 is an issue with that) to the WinRT objects in order to avoid circular references, or possibly to indicate failure if the application forgot to unregister (an assert would be better for that...) During development I had to change the lifetime schemes several times and I believe the FAIL_FAST was left in error.

A scenario might be a JS object references a jsi::HostObject (ProjectedRuntimeClass/InterfaceInstance) and calls addEventListener, with a handler (lambda) referencing the JS object ,on the jsi::HostObject holding a strong ref to the WinRT object. Presently, the ProjectedEventRegistrar holds a strong ref to handler since a JS caller wouldn't necessarily hold a strong ref to the handler . The handler might also not hold a strong ref to the JS object or jsi::HostObject in some scenarios either but the event registration should outlast them if the WinRT object is kept alive such as if it is a static factory or held through some other WinRT object. So it is intentional that the handler is not unregistered if the HostObject expires.

It is expected for periodic calls to ProjectedEventRegistrar::CleanupMapIfApplicable to check if objects are alive and free handlers if they are not. If a JS object calls addEventListener on the HostObject to register, but then lets the WinRT object expire by not holding strong refs to it, then when CleanupMapIfApplicable is next called it would trip up on the FAIL_FAST in ProjectedEventInstance::~ProjectedEventInstance because removeEventListener was not called and the ProjectedInterfaceInstance did not do so on destruction. This issue might not be seen for some time as CleanupMapIfApplicable will skip cleanup fewer than five minutes and 100 add operations have occurred. This could also occur for example if one reloads the JS under the right conditions that cause ProjectedEventRegistar to be freed release all ProjectedInterfaceInstance instances.

Currently, there are a lot of tests that run through the same scenario on different tests (e.g. for a boolean type, a numeric type, etc.) and one of those is generally something of "Object" type. We currently use IVector in most scenarios, but that's kind of overkill. We should switch to using something more like IInspectable as the return type or some other small object type (or both?).

Note that there are a large number of non-exclusive interfaces, and this could bloat the projection. Keep performance and size in mind when designing an appropriate solution to this problem.

There's a number of places where I believe we can improve on binary size. A few:

• Reduce the number of string literals by doing more string concatenation at runtime. Most of these are for throwing exceptions, so there's no real downside to doing this as that's not a common path
• Refactor throwing code out into __declspec(noinline) functions. Again, see above. These are not common code paths and the reduction in duplicated code should help out
• Refactor duplicated code in templated code paths into shared non-templated functions where possible. I'm not sure how much of this we can do, but the impact of something like array-like support for collections types causes a fair amount of bloat. We might be able to do better here.

This issue was inherited from my original prototype code and may explain some of the issues where VS tries to drop ARM64 from the sln.

Clean build doesn't work as in WinRTTurboModule.props is not revaluated when building so it will attempt to build the .g.cpp files that exist when compilation starts which would be none with a clean build or a potentially stale list if .g.cpp files are added/removed due to changing the rnwinrt filter params prior to an incremental build. The solution is to generate the list instead with a build target like as follows:

<PropertyGroup>
    <BuildDependsOn>
        RnWinRTAddGeneratedCppFiles;
        $(BuildDependsOn)
    </BuildDependsOn>
</PropertyGroup>

<Target Name="RnWinRTAddGeneratedCppFiles"
        DependsOnTargets="RnWinRTMakeProjections">
    <ItemGroup>
        <_FilesToBuild Remove="@(_FilesToBuild)"/>
        <_FilesToBuild Include="$(GeneratedFilesDir)rnwinrt\*.g.cpp"/>
    </ItemGroup>
    <ItemGroup>
        <ClCompile Include="@(_FilesToBuild)" />
        <FileWrites Include="@(_FilesToBuild)" />
    </ItemGroup>
    <Message Text="GeneratedCppFiles: @(_FilesToBuild)" Importance="$(RnWinRTVerbosity)" />
</Target>

It seems that the code behind tries to take a weak ref to objects we're adding an event handler to. This fails with static events (on activation factories) pretty consistently, but seems like it would also cause issues if an object instance doesn't support the weak ref interface as well.

Hermes has an explicit block to prevent needed functionality on our part. JsWinRT needs the result from Function::createFromHostFunction() to be able to be used as a constuctor. There is already an existing issue on Hermes for this with a potential (but not very performant) workaround we could utilize.

Existing Hermes issue:
react-native-community/discussions-and-proposals#91 (comment)

Update the library and test apps to target RNW 0.64
CC @nichamp - related to #30

I tried consuming the latest NuGet package "20210610.2.0" and am seeing C2370 error compiling the codegen from WinRTTurboModule as static_class_data is forward-declared as extern const in headers, but defines as extern constexpr in cpp files.

For example:

namespace rnwinrt::classes::Windows::ApplicationModel::UserDataAccounts
{
    namespace UserDataAccount
    {
        extern const static_class_data data;
    }

namespace rnwinrt::classes::Windows::ApplicationModel::UserDataAccounts::UserDataAccount
{
    constexpr const static_class_data data{ "UserDataAccount"sv, {}, {}, {} };
}

2>  ...\windows\WinRTTurboModule\Generated Files\rnwinrt\Windows.ApplicationModel.UserDataAccounts.g.cpp(227,43): error C2370: 'rnwinrt::interfaces::Windows::ApplicationModel::UserDataAccounts::IUserDataAccount::data': redefinition; different storage class
2>  ...\windows\WinRTTurboModule\Generated Files\rnwinrt\Windows.ApplicationModel.UserDataAccounts.g.h(60): message : see declaration of 'rnwinrt::interfaces::Windows::ApplicationModel::UserDataAccounts::IUserDataAccount::data'

We currently have a bunch of static function/event/property tests, but they are all on a type that is activatable. Since there is a rather large divide between activatable and non-activatable classes, we should validate static-only types as well.

Although there are only a few of these in the WinRT type system, the projection doesn't handle these gracefully yet.

This is similar to #63, however I'm going to separate this out since it probably requires a much closer look & thought since it likely isn't as easy as changing a few lines of code. From what I can gather, any use of virtual functions will cause some amount of RTTI data to get forced into the binary, even if it's not used anywhere. We can maybe turn off RTTI, however there's a decent chance that might break things we link against (e.g. JSI), so this may not even be addressable.

The PR pipeline publishes NuGet packages to the feed for PR validation using the same versioning and naming scheme as those generated )by CI?) for the completed PRs. To avoid confusion between in-progress work, and "official" packages corresponding to completed PRs, the PR builds should not publish, or should publish using a clearly different naming convention.

There's a fair amount now, but there's certainly room for more. I'll try and add to this list as I think of more:

• Pass array as IVectorView
• Pass array as IIterable
• Observable collections (IObservableVector/IObservableMap)
  • TODO: Also allow passing array as IObservableVector?

E.g. if a file has 'use strict' and then tries to use the "putter", an exception will get thrown by Chakra (although the "putting" will actually succeed) because the set call returns undefined. I'd assume that this is for support of a = b = c syntax so that a and b both get assigned the value of c, e.g. similar to C++'s operator=.

This isn't actually our problem since the virtual function in JSI returns void, which then gets translated into undefined by the JSI implementation. It's unclear if this is a problem with the JSI implementation for Chakra (e.g. perhaps it should just returns what it was given as input) or JSI itself (e.g. have the return value be jsi::Value). This will however affect our consumers, so we should probably track down exactly whose issue it is. It's also unknown to me what the behavior to me is for v8/Hermes

Generic types are generated on-demand. I.e. for scenarios where the type is visible to/usable by JS. We already cover a fair amount of this with the Collections tests, but we should be more explicit about testing specific scenarios. In particular, here are scenarios where we should generate generic specializations:

• For static/non-static property types
• For static/non-static function output arguments and return values (inputs unnecessary since they are passed by JS)
  • NOTE: We should still be able to convert arrays to vector for input args
• For delegate input/output args and return values (inputs necessary since they can be implemented in JS)
• For struct members (i.e. IReference)
• For interface requires
  • Recursively, even if one of the interfaces in the chain is not projected
• For class "inheritance" (also recursively)

There are open compliance tasks that need to be reviewed for your jswinrt repo.

Action required: 4 compliance tasks

To bring this repository to the standard required for 2021, we require administrators of this and all Microsoft GitHub repositories to complete a small set of tasks within the next 60 days. This is critical work to ensure the compliance and security of your microsoft GitHub organization.

Please take a few minutes to complete the tasks at: https://repos.opensource.microsoft.com/orgs/microsoft/repos/jswinrt/compliance

• The GitHub AE (GitHub inside Microsoft) migration survey has not been completed for this private repository
• No Service Tree mapping has been set for this repo. If this team does not use Service Tree, they can also opt-out of providing Service Tree data in the Compliance tab.
• No repository maintainers are set. The Open Source Maintainers are the decision-makers and actionable owners of the repository, irrespective of administrator permission grants on GitHub.
• Classification of the repository as production/non-production is missing in the Compliance tab.

You can close this work item once you have completed the compliance tasks, or it will automatically close within a day of taking action.

If you no longer need this repository, it might be quickest to delete the repo, too.

GitHub inside Microsoft program information

More information about GitHub inside Microsoft and the new GitHub AE product can be found at https://aka.ms/gim or by contacting [email protected]

FYI: current admins at Microsoft include @jkoritzinsky, @AaronRobinsonMSFT, @AdamBraden, @ujjwalchadha, @BenJKuhn, @Scottj1s, @manodasanW, @stevenbrix, @dunhor, @kennykerr, @EricJohnson327, @kythant, @DefaultRyan

Javascript stores numbers in a floating point representation that basically limits precise representation of integers to 56 bits. The Chakra engine had special handling for this case in the JS runtime to preserve all 64 bits unless the client app did math operations on the number. This isn't a viable approach for this projection since it needs to work with other existing JS runtimes.

When doing the refactor work, I learned that [out] arguments need not appear at the end of the function. We should have tests that "interweave" input/output arguments to validate correct behavior.

Should probably also throw in ref - i.e. "fill" - arrays as well.

This should also probably include delegates since their invocation - and therefore implementation - differs from normal functions

Haven't debugged it very far, but it seems the handling of IReference<T>[] ignores the array-ness of the type and treats it like an IReference, which will lead to compilation errors trying to use a com_array<...> as an IReference<...> (E.g. 'com_array' does not have method named 'Value', etc.)

This is especially important for being able to run against React Native Windows in debug mode, rather than using 'debug bundle' mode, which provides much faster iteration.

One potential part of the solution might be providing a code generator that can provide no-op mock stubs to make development of mocks much faster.

Right now we only test implementing delegates in JS and invoking them in native. This should also work the other way around and we should test it. Array inputs/outputs/returns are probably the most interesting things to test here. This should probably "just work" since it's not much different from invoking functions normally.