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

fixed react-native-windows path definition #357

Closed
wants to merge 2 commits into from
Closed

fixed react-native-windows path definition #357

wants to merge 2 commits into from

Conversation

Hirurgo
Copy link
Contributor

@Hirurgo Hirurgo commented Mar 14, 2018

- About our project
In our project we use reactive-native-device-info and some other "Native" packages, and we provide automatic CI process for three platforms: IOS, Android and Windows UWP.
We had some difficulties with setting up CI for Windows UWP.

- Notice
Before describing the problem and solution, I want to warn you that I'm not a professional windows developer, and maybe my actions were not the most faithful.

- About problem
RNDeviceInfo requires a reference to the react-native-windows package. And there are two possible locations for the folder with a react-native-windows package:

First:
react-native-windows can be located in the node_modules folder in the most react-native-device-info, then in the file
react-native-device-info / windows / RNDeviceInfo / RNDeviceInfo.csproj on the line 116 of code should have such path
<ProjectReference Include="..\..\node_modules\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj">
(The folder react-native-windows lies on two levels above and in the folder node_modules)

Second
The folder react-native-windows can be located next to the folder react-native-device-info in node_modules of another project (as in our case)
In this case, the path to the reactive-native-device-info / windows / RNDeviceInfo / RNDeviceInfo.csproj on the 116 line of code should have this path
<ProjectReference Include = ".. \ .. \ .. \ react-native-windows \ ReactWindows \ ReactNative \ ReactNative.csproj">
(The folder react-native-windows is three levels higher)

If we build our project using Visual Studio 2017, the process of restoring references to the dependency modules occurs (it's incomprehensible to me)

And path
. .\..\node_modules\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj
Automatically turns into
..\..\..\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj

And projects are going well!

But we use an automatic CI system based on the msbuild console utility from Microsoft

And although Visual Studio is based on msbuild, when assembling through it, there is no restoring of references (for two days of searching I have not found the answer why this works exactly the same way and how the mechanism for restoring links in the msbuild console starts)

- Solution
In the windows / RNDeviceInfo / RNDeviceInfo.csproj file, replace the root path of the variable

<ProjectReference Include="..\..\node_modules\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj"> 
<ProjectReference Include="$(ReactWindowsRoot)\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj"> 

And add a special condition for updating this route

< PropertyGroup >
  <ReactWindowsRoot>..\..\node_modules</ReactWindowsRoot> 
</PropertyGroup> 
<PropertyGroup Condition="Exists('..\..\..\react-native-windows')"> 
  <ReactWindowsRoot>..\..\..</ReactWindowsRoot> 
</PropertyGroup> 

This change guarantees us the correct path definition to react-native-windows
P.S. - and the build-reactive-native-device-info requires WinRTXamlToolkit because in the file windows / RNDeviceInfo / project.json, depending on which we wrote the line "WinRTXamlToolkit": "2.0.0"

P.S.S. - Thank you for a good package if you have ideas and how best to solve or bypass this problem I will be glad to discuss them

@machour
Copy link
Contributor

machour commented Mar 14, 2018

Hi @Hirurgo and thank you so much for taking the time to send this PR and the related explanations.

For starters, like you, I'm not a UWP developer, and in fact I just finished this morning setting up a Windows 10 to be able to maintain this library :)

Some members of the react-native-windows project are usually hanging out on Reactiflux in #react-native-platforms. Maybe you could join there and try to understand the issue you're facing.

From what I've read, you are correct, and we're missing an additional "../" in the path in the csproj file.
I'll test your PR as soon as I have a working environment for tests and will get back to you.

Meanwhile, could you please re-explain the addition of WinRTXamlToolkit to the .json file? I'm not sure I understood what was the problem with that.

By the way, if any other native packages you used, with a windows implementation, is publicly available, I'd really love to check them out.

Thank you again for reaching out!

@machour
Copy link
Contributor

machour commented Mar 15, 2018

By the way, I published 0.20.0 with more Windows support yesterday:
https://github.com/rebeccahughes/react-native-device-info/releases/tag/v0.20.0

Since you're the only one I know of with a production application running on UWP, would you mind giving it a try and tell me if I broke anything? It was my first attempt at C# 😅

@Hirurgo
Copy link
Contributor Author

Hirurgo commented Aug 10, 2018

@machour I apologize for my long absence in solving this problem, as before with the current version of 0.22.3 I have the same error
image

@Brynjulf
Copy link
Contributor

@machour , I'm struggling with the same problem that the ReactNative.csproj file was not found.

Here's a package I'm using that is currently not giving any errors or trouble:
https://github.com/itinance/react-native-fs

@Hirurgo
Copy link
Contributor Author

Hirurgo commented Aug 14, 2018

@Brynjulf Thanks, for your comment, react-native-fs have fixed and correct path to react-native-windows
134 line from RNFS.csproj:
<ProjectReference Include="..\..\..\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj">

@Brynjulf
Copy link
Contributor

Brynjulf commented Aug 17, 2018

@Hirurgo, @machour To follow up a bit more on the OP, WinRTXamlToolkit": "2.0.0 is something that visual studio complains about missing when attempting to build the solution. This is not necessary to reference if you instead upgrade TargetPlatformVersion (and TargetPlatformMinVersion) to 10.0.15063.0 and use the new way of referencing[1]. This would mean that you need to remove the project.json and its reference inside RNDeviceInfo.csproj, this would also remove another error about uap mismatch generated from specifying the framework and runtimes inside the project.json.

Below is my entire patch file for running my project with react-native-device-info, it was generated using patch-package[2]. This patch enables builds in visual studio, terminal or remote (we're using vsts to deploy). It also works for debug, release and bundled release/debug.

patch-package
--- a/node_modules/react-native-device-info/windows/RNDeviceInfo/RNDeviceInfo.csproj
+++ b/node_modules/react-native-device-info/windows/RNDeviceInfo/RNDeviceInfo.csproj
@@ -11,11 +11,16 @@
     <AssemblyName>RNDeviceInfo</AssemblyName>
     <DefaultLanguage>en-US</DefaultLanguage>
     <TargetPlatformIdentifier>UAP</TargetPlatformIdentifier>
-    <TargetPlatformVersion>10.0.14393.0</TargetPlatformVersion>
-    <TargetPlatformMinVersion>10.0.10586.0</TargetPlatformMinVersion>
+    <TargetPlatformVersion>10.0.15063.0</TargetPlatformVersion>
+    <TargetPlatformMinVersion>10.0.15063.0</TargetPlatformMinVersion>
     <MinimumVisualStudioVersion>14</MinimumVisualStudioVersion>
     <FileAlignment>512</FileAlignment>
     <ProjectTypeGuids>{A5A43C5B-DE2A-4C0C-9213-0A381AF9435A};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReactWindowsRoot>..\..\node_modules</ReactWindowsRoot>
+    <RestoreProjectStyle>PackageReference</RestoreProjectStyle>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)' != 'Development'">
+    <ReactWindowsRoot>..\..\..</ReactWindowsRoot>
   </PropertyGroup>
   <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
     <PlatformTarget>AnyCPU</PlatformTarget>
@@ -102,10 +107,6 @@
     <UseVSHostingProcess>false</UseVSHostingProcess>
     <ErrorReport>prompt</ErrorReport>
   </PropertyGroup>
-  <ItemGroup>
-    <!-- A reference to the entire .Net Framework and Windows SDK are automatically included -->
-    <None Include="project.json" />
-  </ItemGroup>
   <ItemGroup>
     <Compile Include="RNDeviceInfoPackage.cs" />
     <Compile Include="RNDeviceInfoModule.cs" />
@@ -113,11 +114,16 @@
     <EmbeddedResource Include="Properties\RNDeviceInfo.rd.xml" />
   </ItemGroup>
   <ItemGroup>
-    <ProjectReference Include="..\..\node_modules\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj">
-      <Project>{c7673ad5-e3aa-468c-a5fd-fa38154e205c}</Project>
+    <ProjectReference Include="$(ReactWindowsRoot)\react-native-windows\ReactWindows\ReactNative\ReactNative.csproj">
+      <Project>{C7673AD5-E3AA-468C-A5FD-FA38154E205C}</Project>
       <Name>ReactNative</Name>
     </ProjectReference>
   </ItemGroup>
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NETCore.UniversalWindowsPlatform">
+      <Version>6.1.7</Version>
+    </PackageReference>
+  </ItemGroup>
   <PropertyGroup Condition=" '$(VisualStudioVersion)' == '' or '$(VisualStudioVersion)' &lt; '14.0' ">
     <VisualStudioVersion>14.0</VisualStudioVersion>
   </PropertyGroup>
deleted file mode 100644
--- a/node_modules/react-native-device-info/windows/RNDeviceInfo/project.json
+++ /dev/null
@@ -1,16 +0,0 @@
-{
-  "dependencies": {
-    "Microsoft.NETCore.UniversalWindowsPlatform": "5.2.2"
-  },
-  "frameworks": {
-    "uap10.0": {}
-  },
-  "runtimes": {
-    "win10-arm": {},
-    "win10-arm-aot": {},
-    "win10-x86": {},
-    "win10-x86-aot": {},
-    "win10-x64": {},
-    "win10-x64-aot": {}
-  }
-}
\ No newline at end of file

Visual studio is more forgiving when finding references, and i suspect it doesn't solely search for path reference but also project id when you've linked up dependencies. It would make sense given that you can still run failed builds (because of certain errors) from visual studio, but not from the terminal.

I hope this helps.

Reference:

  1. https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files
  2. https://github.com/ds300/patch-package

@mikehardy
Copy link
Collaborator

@Brynjulf - a fellow patch-package user, I love that package. Would you be willing to confirm if your patch is still in use by you and working? And that it is all that is necessary to resolve this issue?

If so, I'll copy it out to a diff and merge it in (or you could propose a PR and I'll merge it)

@Brynjulf
Copy link
Contributor

@mikehardy, it's an awesome package albeit a little rough when using it while changing OS'. We currently have 4 apps that are using the dynamic path patch.

I made a pull request for you here

@machour
Copy link
Contributor

machour commented Mar 28, 2019

Closed in favor of #608

@machour machour closed this Mar 28, 2019
mikehardy pushed a commit that referenced this pull request Mar 28, 2019
@mikehardy, 

## Description

This is the required change that is necessary for windows to resolve the reference path in different environments (outside of visual studio which automatically resolves references).

## Compatibility

| OS      | Implemented |
| ------- | :---------: |
| iOS     |    ❌     |
| Android |    ❌     |
| Windows |    ✅     |

## Checklist

<!-- Check completed item: [X] -->

* [ ] I have tested this on a device/simulator for each compatible OS
* [ ] I added the documentation in `README.md`
* [x] I mentioned this change in `CHANGELOG.md`
* [ ] I updated the typings files (`deviceinfo.d.ts`, `deviceinfo.js.flow`)
* [ ] I updated the web polyfill (`web/index.js`)
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.

4 participants