Rectify the scopes in the builder #19932
Merged
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
PureWeen
requested changes
Jan 16, 2024
mattleibow
force-pushed
the
dev/fix-scopes
branch
3 times, most recently
from
c3e3c59 to
a1fe5eb
Compare
mattleibow
commented
Jan 17, 2024
Comment on lines
-51
to
+55
| var window = new MauiWindow(Services.GetRequiredService<Page>()) | ||
| var services = activationState!.Context.Services; | ||
| var window = new MauiWindow(services.GetRequiredService<Page>()) |
There was a problem hiding this comment.
This has to be changed because the IServiceProvider in the app's ctor is actually the root povider and does not have a window dispatcher.
The page uses a VM that uses the dispatcher. However, the services coming in here via the activation state is a window provider and does have the dispatcher.
mattleibow
force-pushed
the
dev/fix-scopes
branch
from
a1fe5eb to
5852f04
Compare
mattleibow
commented
Jan 17, 2024
mattleibow
force-pushed
the
dev/fix-scopes
branch
from
5852f04 to
09a9b04
Compare
PureWeen
reviewed
Jan 17, 2024
mattleibow
force-pushed
the
dev/fix-scopes
branch
5 times, most recently
from
312a09f to
0994b7a
Compare
|
This PR needs to be merged AFTER #20014 as I want to use keyed services for most cases but also add a fallback for services that do not support keyed services |
mattleibow
force-pushed
the
dev/fix-scopes
branch
2 times, most recently
from
64fd3d6 to
03dfa96
Compare
PureWeen
requested changes
Jan 23, 2024
|
ok, i guess for net 8 we can't even register keyed services then. Makes me sad, but we will have to use the internal/hidden thing for now. I am keeping the lookup for keyed services so it is actually overridable - like in our tests - but it won't use it by default. |
mattleibow
force-pushed
the
dev/fix-scopes
branch
4 times, most recently
from
6e22a10 to
10e90b1
Compare
PureWeen
approved these changes
Jan 23, 2024
PureWeen
reviewed
Jan 23, 2024
| return scopedContext; | ||
| } | ||
|
|
||
| public static IMauiContext MakeWindowScope(this IMauiContext mauiContext, NativeWindow platformWindow, out IServiceScope scope) | ||
| { | ||
| // Create the window-level scopes that will only be used for the lifetime of the window | ||
| // TODO: We need to dispose of these services once the window closes |
|
/rebase |
github-actions
bot
force-pushed
the
dev/fix-scopes
branch
from
10e90b1 to
f1fd6f2
Compare
|
/rebase |
rmarinho
added a commit
that referenced
this pull request
Jan 29, 2024
* Add Obsolete tag for old IndexOf (#20068) * Split the InputTransparency tests (#19925) * Bump Xamarin.UITest to 4.3.4 (#20067) * Bump Xamarin.UITest to 4.3.4 * Update NUnit * Restore dotnet tool (#20106) * Do not use underscores in the ApplicationId (#19377) * Do not use underscores in the ApplicationId Underscores are not supported on Windows * Update DotnetInternal.cs * [WinUI] Add workaround for Connectivity check on Win10 (#19261) * Implemented a Win10 work-around for connectivity thread issue Reimplement `ConnectivityImplementation.ConnectionProfiles` to use native .net core APIs * Remove prop bag ID * Replace network availability changed event with native .net API * Remove explicit file include * Fix file naming * Implemented a Win10 work-around for connectivity thread issue Reimplement `ConnectivityImplementation.ConnectionProfiles` to use native .net core APIs * Remove prop bag ID * Replace network availability changed event with native .net API * Remove explicit file include * Fix file naming * * Revert change to remove `Windows.Networking.Connectivity.NetworkInformation.NetworkStatusChanged` * Update Connectivity.uwp.cs --------- Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> * [Windows] Adjust recycle check in ItemContentControl (#20079) * Adjust recycle check in `ItemContentControl` to use Content property * Fix confusing line * Rectify the scopes in the builder (#19932) * Update System.Drawing.Common (#20122) * Enable building WASDK Self-Contained packaged apps (#20019) * Translucent and Transparent NavigationBar on iOS (#19204) * Changes for enabling translucent navigation bar ios * Add UITest for NavigationPage Safe Area Translucence * remove UIKit * Move UITest from gallery to Issues * push a new page for UITests, consolidate code, and save shadowimage * Changes for enabling translucent navigation bar ios * Add UITest for NavigationPage Safe Area Translucence * remove UIKit * Move UITest from gallery to Issues * make the extension method * use the background color alpha for translucent * use same alpha logic for pre ios 15 * accidently added testing comments * add more UITests for the different NavigationPage and Flyout Page scenarios * use additionalsafeareainsets for the secondary toolbar * missing new line * only run the secondary toolbar offset when we have a secondary toolbar * use existing childViewControllers * remove the shadow stuff and simplify the expression with null * style and fixes from merge conflicts * standardAppearance and scrolledgeappearance should be kept in * changes after talking with Shane * change shell behavior to be more like navrenderer with preiOS13 * move code if we are transparent pre13 shell * remove new lines * add screenshot tests * be able to remove and add secondarybar additionalsafeareas * reset the xaml on the Sandboc MainPage (#20082) * [Android] Fix gif animation initial state (#14295) * Fix gifs initial animation state on Android * Device tests * Auto-format source code * Updated sample * Updated device tests * Refactor code * Update src/Core/src/Handlers/Image/ImageHandler.Android.cs Co-authored-by: Matthew Leibowitz <mattleibow@live.com> * Remove unnecessary change * Fix build errors * Merge branch 'main' into fix-7019 * Fix merge mistake --------- Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com> Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> * Enable Windows Image device tests (#20167) * [X] allow x:Type extension for BPConverter (#18540) - fixes #18324 * Source Generator Performance Improvements (#19990) * Source Generator Performance Improvements: - Reversed lookup order (Extension second) - Introduced type cache ``` PERF PROGRESS: SourceGen - Maui.Controls.Sample (262 XAML files) 1) Unchanged: - 15640 GetTypeByMetadata calls - 223s 2) Extensions lookup in XmlTypeXamlExtensions.GetTypeReference() second - 6232 GetTypeByMetadata calls (~60% reduction) - 90s (~60% reduction) 3) With type cache - 203 GetTypeByMetadata calls (~97% reduction => ~99% total reduction) - 6s (~93% reduction => ~97% total reduction => 37 times faster!) ``` * - Set uinitial lookupNames capacity to 2 since there won't be more than 2 * Added appium UITest for FlyoutNavigationBetweenItemsWithNavigationStacks (#19444) Fixes #16675 * Add better exception for missing Maps on Windows (#19046) * Add better exception for missing Maps on Windows * Update AppHostBuilderExtensions.cs * [ci] Add missing demands (#20183) * Add comments to internal methods for XAML Hot Reload usage (#20215) * Add comments to internal methods for XAML Hot Reload usage * spacing * Adding Mobile tag to MAUI project templates (#20191) Co-authored-by: Luke Westendorf <lukewest@microsoft.com> --------- Co-authored-by: Tim Miller <drasticactions@users.noreply.github.com> Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: Mike Corsaro <corsarom@gmail.com> Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com> Co-authored-by: TJ Lambert <50846373+tj-devel709@users.noreply.github.com> Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com> Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> Co-authored-by: Marco Goertz <mgoertz@microsoft.com> Co-authored-by: MSLukeWest <42553283+MSLukeWest@users.noreply.github.com> Co-authored-by: Luke Westendorf <lukewest@microsoft.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description of Change
An alternate alternative to #18492 and an alternative to #19593
Alteratives:
This PR makes sure that our scopes are correct:
I would have preferred to have a singleton service for the app and a scoped service for window, but it appears the default DI container does not support the same type as BOTH a singleton AND a scoped. One thought would be to use keyed services but this does not yet appear to be 100% supported in the IoC container libraries that are used in a MAUI app. For now we just register a placeholder type to hold the singleton dispatcher and we use that.
The issue is that an app may not only have different dispatchers per window, but also for the app. In all existing apps today, there is a single dispatcher for all things. However, WinUI used to - and may in future - have different dispatchers for each window.
This could mean that at startup, the app and window have the same dispatcher, but if a new window was created, it would have a new, second dispatcher. And if the first window was closed, the app would retain the first dispatcher. As a result, we could have different dispatchers for the app and for the window. This is academic now since WinUI dropped the support for different window-threads.
Issues Fixed