microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.26k stars 1.14k forks source link

Code Cleanup: Bundle.props can be moved into Bundle.targets #5048

Open dannyvv opened 4 years ago

dannyvv commented 4 years ago

At the moment the bundle.props is assumed to be imported after all the users settings. Typically .props are imported at the top of the project and set the defaults.

This issue tracks:

JunielKatarn commented 4 years ago

Does Bundle.props contain targets, or properties and item definitions?

asklar commented 4 years ago

so we originally had everything in one file but I had asked that we split things up into properties that we want to be able to be overridable by the project, and the targets which we dont expect people to want to override, so that's how we ended up with the current layout.

dannyvv commented 4 years ago

There are various ways in msbuild to go about this. MsBuild properties are just a simple recursive descent macro expansion. So there are two ways to deal with defaults: a) You unconditionally set them at the top of the file (import the props before the user properties) <Foo>MyDefault</Foo> b) You conditinoally set them when not set to the default after the user had a chance to set them: <Foo Conditoin="'$(Foo) == ''" />

For msbuild all files are the same and extensions don't matter. It is just a convention of .props for property groups and .targets for targets. ItemGroups, ItemDefinitionGroups etc all fall in between as they don't have a real file...

If you want to use public vs private fields. The convention has become as pioneerd by the cpp team to previs the name of the properties, items, targets with an undescore.

So my inference when looking at the code was If we use style 'b' for the defaults, then they might as well be in the targets. Else we should probably switch to style 'a' and place the .props import at the top of the file.