microsoft / react-native-winrt

Windows Runtime projection for React Native for Windows
MIT License
85 stars 11 forks source link

Duplicate winrtturbomodule files in repo #160

Open asklar opened 2 years ago

asklar commented 2 years ago

there is a copy of the turbomodule in the samples and another one in sdk\react\windows however the contents of both are different (see e.g. differences in the vcxproj where the one under sdk includes Microsoft.ReactNative.Uwp.CppLib.targets in a different place as the one in the sample).

dunhor commented 2 years ago

The vcxproj in the samples and the vcxprojs in both the tests and sdk directory are going to be different because the tests/sdk projects don't use the nuget package, unlike the sample. So, there's naturally going to be differences between the two.

Furthermore, the sdk project is kind of a franken-project since it's really just a build test and pulls in RNW strictly because it's necessary for JSI. These projects have also evolved over time as RNW releases have changed the way that projects - and specifically turbo modules - work/are built, which has had a greater impact on the sdk project specifically as it previously really only needed the JSI include path.

I'm sure it's possible to get the vcxprojs to match up better, but so far it's mostly been an "if it ain't broke" type of situation. Is there a functional issue here, or is it more of a consistency/confusion sort of thing?

asklar commented 2 years ago

ah got it, thanks @dunhor - I missed that this was a test, as it has sdk in its name :) Could we maybe put the sdk folder under a tests folder or similar?