search

microsoft / react-native-windows

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

All playground solutions share one ExperimentalFeatures.props, which confuses autolinking #8591

Open jonthysell opened 3 years ago

jonthysell commented 3 years ago

All of the playground solutions are in the same folder, and share the same ExperimentalFeatures.props file, which uses msbuild conditionals to set the WinUI2xVersion. To fix #8431 in PR #8589, autolinking just looks at the file to find the version, and can't apply the conditional logic that msbuild would.

Before adding picker, all 3 playgrounds had autolinking disabled. With picker added in #8432, autolinking was finally enabled for 2/3 playgrounds. With the #8589 fix, the result is flipped - now the lone playground works and the other two don't.

Option one: We could just move the solution files into their own folders so they each get their own ExperimentalProps.props file, and stop using conditionals. This would more closely align how we expect people to use the file, and not give playground's special setup precedence. The downside is that this would require updating the pipelines, along with all the general "loss of productivity / maintainability" of moving files.

Option two: We revert the autolink check behavior so that the process no longer sets the exit code to a failure. The original design was to just give a warning, not fail the build, and devs could decide for themselves if they wanted to resolve it by running autolinking manually. This would let all the playgrounds build and not care that the check is complaining because of the conditional logic in ExperimentalFeatures.props. The downside would be when we get used to ignoring warnings and not run autolinking when we should, just because "well it builds, right?".

Option three: If we really want to support playground's setup, a more involved fix might be to add an msbuild target that lets us output the final evaluation of msbuild variables, so we could actually apply the msbuild conditional logic and get the correct result. Then we could stop reading the ExperimentalProps.props file directly, but instead get the correct version for all 3 playgrounds. Such functionality might be more useful in other situations. The main downside would be the amount of effort required, and that this might be quite a slow to inject into autolinking.

jonthysell commented 3 years ago

I am currently prototyping option 3 with some success

chrisglein commented 2 years ago

Moved out of tracking against 0.68 release as that it needs to be build on top of a PR that's still in draft and we're just a couple weeks out from 0.68. This wasn't meant to target a specific release, but was tracked so based on comments of active prototyping. Moving to track against Backlog but leaving assigned.