-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Handle content hiding separately for hot restart scenarios #6341
Conversation
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
@captainsafia can you ping me on teams to chat about this? Our thinking was that https://github.com/dotnet/maui/pull/6341/files#diff-a403e09c8175f7c626397dc3695070bc822b977346468fe4cc11504388bbc68bL21 would take care of this. Do you happen to/can get a binary log for this scenario? |
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.
LGTM. Kicked off a re-run of the failing CI steps
Our thinking was that https://github.com/dotnet/maui/pull/6341/files#diff-a403e09c8175f7c626397dc3695070bc822b977346468fe4cc11504388bbc68bL21 would take care of this. Synced with Javier offline. We're taking this fix in as a stopgap and will work to address the core issue that required the workaround in the first place in the Xamarian hot reload implementation. |
@captainsafia Looking at the timeline of the merge, I'm supposing that the fix didn't land in time for 17.2 Preview 5? I recently encountered this on a local build. Thanks! |
Correct, this will be part of the next release. |
Is there a fix for this? I'm on the absolute latest on both sides and still experience this issue. Can anyone identify a path to a solution? Thanks |
@captainsafia Is this fix shipped? :) |
@PBTests @vallgrenerik Would one of you mind opening a new issue for this? In particular, it would be helpful to know what combination of VS and iOS versions you are on. |
Description of Change
We've been receiving bug reports that an issue that was resolved by changes to MSBuild in 114d30f still persists in hot reload scenarios or iOS. Both the
_CollectBundleResources
and_CollectHotRestartBundleResources
targets are run during a hot restart build but because of the constraint on target ordering outlined here the hiding/restoring only happens on the first target that is run (_CollectBundleResources
).So the current ordering is:
_CollectBundleResources
runsTo resolve this issue, we need to invoke the hiding/restore targets independently for the
_CollectBundleResources
and_CollectHotRestartBundleResources
. The new sequence is as follows:HideContentFromiOSBundleResources
hides the content_CollectBundleResources runs
RestoreHiddeniOSContent
restores the contentHideContentFromiOSHotRestartBundleResources
hides the content_CollectHotRestartBundleResources
runsRestoreHiddeniOSHotRestartContent
restores the contentIssues Fixed
Fixes #5245