-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Clean Up Directory Logic to Fix Publish #4496
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "type": "prerelease", | ||
| "comment": "Clean Up Directory Logic to Fix Publish", | ||
| "packageName": "react-native-windows", | ||
| "email": "ngerlem@microsoft.com", | ||
| "dependentChangeType": "patch", | ||
| "date": "2020-04-03T23:21:58.354Z" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,10 +43,7 @@ | |
| <UseDotNetNativeToolchain>true</UseDotNetNativeToolchain> | ||
| </PropertyGroup> | ||
| <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" /> | ||
| <PropertyGroup> | ||
| <ReactNativeWindowsDir Condition="'$(ReactNativeWindowsDir)' == ''">$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), 'node_modules\react-native-windows\package.json'))\node_modules\react-native-windows\</ReactNativeWindowsDir> | ||
| </PropertyGroup> | ||
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\ReactPackageDirectories.props" /> | ||
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\ReactDirectories.props" /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming this is the same as
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. Once these are converted from ReactUwp to Microsoft.ReactNative we can keep everything tucked into vnext as well. |
||
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\ReactCommunity.cpp.props" /> | ||
| <Import Project="$(ReactNativeWindowsDir)PropertySheets\React.Cpp.props" /> | ||
| <!-- Include Warnings.props after Microsoft.Cpp.props to change default WarningLevel --> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,9 +91,6 @@ | |
| $(FollyDir); | ||
| $(ReactNativeWindowsDir)\ReactWindowsCore\pch; | ||
| $(ReactNativeWindowsDir)\ReactWindowsCore\tracing; | ||
| $(ProjectDir); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not directly related to the PR, but these shouldn't be here. ProjectDir will already be included, and the latter two allow consuming from build output while building which is dangerous and not really that useful. |
||
| $(GeneratedFilesDir); | ||
| $(IntDir); | ||
| %(AdditionalIncludeDirectories) | ||
| </AdditionalIncludeDirectories> | ||
| <AdditionalIncludeDirectories Condition="'$(USE_HERMES)'=='true'">$(HERMES_Package)\installed\$(VcpkgTriplet)\include\;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this already included in
vnext\Directory.Build.props?I see this project lives outside
vnext. Can you confirm it's explicitly included because of this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playground (non-win32) was able to correctly build locally, meaning it was able to pick up the properties. It it wasn't included, it would fail pretty hard.