Handle changes to ExcludeFromCurrentConfigurationProperty #9287
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change requires a good bit of background. The Solution Explorer builds its tree from the evaluation model of the "active" configuration and only the active configuration, and there can only be one active configuration. Items (source files, resource files, etc.) that aren't part of the active configuration won't be displayed. In MAUI projects, the items under the "Platforms" folder are meant to be platform-specific (by which we really mean target framework-specific), with each relevant platform getting its own subfolder. Items in the "Android" subfolder should not be included in the "iOS" build, for example. The most straightforward way to approach this would be to conditionally include the various `<Compile>` items under Platforms based on the target framework. That would work correctly for the purposes of the build, but means that not all of the items would display in the Solution Explorer--only those belonging to whatever platform is associated with that "active" configuration. Instead, the items are included for *all* target frameworks during MSBuild evaluation, and then the extraneous ones are removed by targets during the build. This ensures they always appear in Solution Explorer and the build still works as expected. However the language service integration in the .NET Project System utilizes both evaluation and design-time data and has to reconcile the two as best it can. One thing it can't handle on its own are items that are present in evaluation but removed by a target. To help with this, we added the concept of `ExcludeFromCurrentConfiguration` metadata for items; we can use this to tell the language service integration to _pretend_ the item doesn't exist in evaluation even though it does. The MAUI .props and .targets end up defining the items under the Platforms folder in all configurations, but the metadata on those items varies depending on whether or not the item should actually be passed to the Language Service. It's important to step back at this point and call out that there are a couple layers of workarounds going on here: MAUI includes the items in every configuration to work around the fact that CPS can't build the Solution Explorer tree from multiple target frameworks, and the language service needs `ExcludeFromCurrentConfiguration` metadata to work around the fact that the items are in every configuration. And now we can finally talk about bug AB#1895917. The above scheme works well enough when the `ExcludeFromCurrentConfiguration` metadata is properly applied to the items. The problem is that the metadata is added by a .targets file (Microsoft.Maui.Controls.SingleProject.targets) that comes from a NuGet package (Microsoft.Maui.Controls.Build.Tasks). If an evaluation occurs before NuGet restore runs and the Language Service integration processes that evaluation, we will see the items without the metadata. This means that we will pass Android-specific source files along as if they were also part of the Mac, iOS, and Windows builds--and do the same for every other platform-specific file. This will lead to spurious errors in the IDE (e.g. the Windows build complaining about the Andoid APIs as those aren't defined). More to the point of this bug, Hot Reload won't work because the IDE will notice the discrepancy between the types it knows about and what is actually in the built assembly and think that it missed some edits. The fix here is to also look out for changes in the `ExcludeFromCurrentConfiguration` metadata. If a file was previously included and we get a new evaluation saying it is excluded, we need to tell the Language Service to remove it. And if it was previously excluded and now is included, we need to tell the Language Service to add it. This way even if we provided the "wrong" data to the Language Service initially, once NuGet restore has run and we process the next evaluation we can fix everything up.
tmeschter
commented
Oct 17, 2023
|
|
||
| // TODO: Check for changes in the metadata indicating if we should ignore the file | ||
| // in the current configuration. | ||
| HandleEvaluationMetadataChange(context, includePath, previousMetadata, currentMetadata, isActiveContext, logger); |
There was a problem hiding this comment.
Note that UpdateInContextIfPresent has been renamed to HandleEvaluationMetadataChange to reflect that it does more than update already-present items.
drewnoakes
added
the
Feature-Language-Service
drewnoakes
approved these changes
Oct 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change requires a good bit of background.
The Solution Explorer builds its tree from the evaluation model of the "active" configuration and only the active configuration, and there can only be one active configuration. Items (source files, resource files, etc.) that aren't part of the active configuration won't be displayed.
In MAUI projects, the items under the "Platforms" folder are meant to be platform-specific (by which we really mean target framework-specific), with each relevant platform getting its own subfolder. Items in the "Android" subfolder should not be included in the "iOS" build, for example.
The most straightforward way to approach this would be to conditionally include the various
<Compile>items under Platforms based on the target framework. That would work correctly for the purposes of the build, but means that not all of the items would display in the Solution Explorer--only those belonging to whatever platform is associated with that "active" configuration.Instead, the items are included for all target frameworks during MSBuild evaluation, and then the extraneous ones are removed by targets during the build. This ensures they always appear in Solution Explorer and the build still works as expected.
However the language service integration in the .NET Project System utilizes both evaluation and design-time data and has to reconcile the two as best it can. One thing it can't handle on its own are items that are present in evaluation but removed by a target. To help with this, we added the concept of
ExcludeFromCurrentConfigurationmetadata for items; we can use this to tell the language service integration to pretend the item doesn't exist in evaluation even though it does. The MAUI .props and .targets end up defining the items under the Platforms folder in all configurations, but the metadata on those items varies depending on whether or not the item should actually be passed to the Language Service.It's important to step back at this point and call out that there are a couple layers of workarounds going on here: MAUI includes the items in every configuration to work around the fact that CPS can't build the Solution Explorer tree from multiple target frameworks, and the language service needs
ExcludeFromCurrentConfigurationmetadata to work around the fact that the items are in every configuration.And now we can finally talk about bug AB#1895917. The above scheme works well enough when the
ExcludeFromCurrentConfigurationmetadata is properly applied to the items. The problem is that the metadata is added by a .targets file (Microsoft.Maui.Controls.SingleProject.targets) that comes from a NuGet package (Microsoft.Maui.Controls.Build.Tasks). If an evaluation occurs before NuGet restore runs and the Language Service integration processes that evaluation, we will see the items without the metadata. This means that we will pass Android-specific source files along as if they were also part of the Mac, iOS, and Windows builds--and do the same for every other platform-specific file. This will lead to spurious errors in the IDE (e.g. the Windows build complaining about the Andoid APIs as those aren't defined). More to the point of this bug, Hot Reload won't work because the IDE will notice the discrepancy between the types it knows about and what is actually in the built assembly and think that it missed some edits.The fix here is to also look out for changes in the
ExcludeFromCurrentConfigurationmetadata. If a file was previously included and we get a new evaluation saying it is excluded, we need to tell the Language Service to remove it. And if it was previously excluded and now is included, we need to tell the Language Service to add it. This way even if we provided the "wrong" data to the Language Service initially, once NuGet restore has run and we process the next evaluation we can fix everything up.Microsoft Reviewers: Open in CodeFlow