Release bundle builds should have --dev false flag #4603
Conversation
acoates-ms
added
the
AutoMerge
|
Hello @acoates-ms! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
| --> | ||
| <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
| <Import Project="$(MSBuildThisFileDirectory)\Bundle.props"/> | ||
|
|
There was a problem hiding this comment.
props are supposed to be Imported by the app project. The idea is in the app project you'd do
Import props
override/add your own properties
Import targets
There was a problem hiding this comment.
@asklar
I moved some things around. But I'm not 100% sure this is correct. Should the BundleCommand be defined in the props file at all? Or should it just be moved into the targets file? The way it currently is, if you're overriding something like BundleEntryFile you have to do it before you import the props, which is not how you described it above.
ghost
added
the
Needs: Author Feedback
ghost
removed
the
Needs: Author Feedback
…ct, since RNW could be hoisted
acoates-ms
force-pushed
the
releasebundle
branch
from
fae3141 to
3abe545
Compare
asklar
left a comment
asklar
left a comment
There was a problem hiding this comment.
main 3 pieces of feedback:
- include props early in the project (before propertygroups are defined)
- make sure ProjectDir is defined in cmdline msbuild scenarios too not just VS
- see if you can unify Bundle.targets and Bundle.cpp.targets. I think even if you combine the two, it'd be fine, the C# properties would get picked up by the C# projects and the C++ properties by the C++ projects
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.NETCore.UniversalWindowsPlatform"> | ||
| <Version>6.2.8</Version> | ||
| <Version>6.2.10</Version> |
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\Bundle.targets" /> | ||
| <Import Project="$(MSBuildExtensionsPath)\Microsoft\WindowsXaml\v$(VisualStudioVersion)\Microsoft.Windows.UI.Xaml.CSharp.targets" /> | ||
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\Bundle.props" /> | ||
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\Bundle.targets" /> |
There was a problem hiding this comment.
here in this C# project we use Bundle.targets but in C++ we use Bundle.cpp.targets. Why the difference? Can we just use one targets file and if we need to have different behavior it can figure it out for us?
| npx --no-install yarn run bundle-cpp | ||
| </BundleCommand> | ||
| </PropertyGroup> | ||
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\Bundle.props" /> |
There was a problem hiding this comment.
normally you'd put all the imported props files before any propertygroup. I think this is the same end result in this case as you don't seem to be overwriting/overriding anything in the bundle.props, but it'd be worth sticking to the layout
| </ItemGroup> | ||
|
|
||
| <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" /> | ||
| <Import Project="$(ReactNativeWindowsDir)\PropertySheets\Bundle.props" /> |
There was a problem hiding this comment.
move up before targets (really before the first PropertyGroup)
| <BundleContent Condition = " '$(BundleContent)' == '' and '$(BundleContentRoot)' != '' ">$(BundleContentRoot)\**\*</BundleContent> | ||
|
|
||
| <!-- Root directory where bundle assets will be copied to, be sure to update BundleContent if you change this --> | ||
| <BundleContentRoot Condition=" '$(BundleContentRoot)' == '' ">$([MSBuild]::NormalizePath('$(ProjectDir)\Bundle'))</BundleContentRoot> |
There was a problem hiding this comment.
I think ProjectDir is a VS thing whereas MSBuildProjectDirectory is the MSBuild property - are you seeing this work on the command line too?
ghost
merged commit 2 participants
Fixes #4276
The default template creates a bundle file when doing a release build ready for the store. But that bundle is a debug bundle. This updates our bundle options in Bundle.props to align with the customization available on android in the default react-native gradle build.
https://github.com/facebook/react-native/blob/894f6b3e744679769ba0f6b83bae89354f0f1914/react.gradle#L170
Also updated the project file of the template to correctly handle react-native-windows being hoisted
Microsoft Reviewers: Open in CodeFlow