Skip to content

Remove calls to scoped services on root provider#19593

Closed
PureWeen wants to merge 3 commits intomainfrom
scoped_service
Closed

Remove calls to scoped services on root provider#19593
PureWeen wants to merge 3 commits intomainfrom
scoped_service

Conversation

@PureWeen
Copy link
Copy Markdown
Member

@PureWeen PureWeen commented Dec 26, 2023

Description of Change

Alternative approach for #18492.

This approach opts for changing current behavior as little as possible for NET8. Instead of changing the behavior of IMauiInitializeScopedService this just stops using it. AFAICT it's not really providing any current value for us. We can just use IMauiInitializeService to achieve the same workaround for WinUI.

The main behavior change here is how to handle the first call to IDispatcher from the service provider scoped to the window.

Behavior Change

Currently in MAUI if you try to retrieve the Dispatcher from the scoped service provider on a background thread it just returns null, but this captures the dispatcher when the window scope is created. Which is probably a better experience.

https://github.com/dotnet/maui/pull/19593/files#diff-500f3422fdff10304272c2e7985ec98941543f788bef70ac1fb1b23776bb1104R49-R55

Added issue here to evaluate the correctness of IMauiInitializeScopedService because this interface currently only initializes once at the application level, it doesn't actually initialize for each scoped service we create.

Issues Fixed

Fixes #11457

builder.Services.AddSingleton<ApplicationDispatcher>();


builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IMauiInitializeService, DispatcherInitializer>());
Copy link
Copy Markdown
Member Author

@PureWeen PureWeen Dec 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ok @mattleibow ?

I can't really tell a difference here between using IMauiInitializeService vs IMauiInitializeScopedService. They are both fired from OnLaunched in MauiWinUIApplication so they both will capture the dispatcher from the main thread

The purpose of these are to initialize at the app level not the "Scoped" windows level so it seems like they should just be IMauiInitializeService

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMauiInitializeService should only run once per app during the MauiAppBuilder.Build() method. The scoped services are supposed to be initialized for each window. However... I see that it is not working like that so I want to see what is really happening and if this is all a bug because we are not doing something...

@Eilon Eilon added the area-core-hosting Extensions / Hosting / AppBuilder / Startup label Dec 29, 2023
@PureWeen
Copy link
Copy Markdown
Member Author

PureWeen commented Jan 5, 2024

/rebase

@PureWeen PureWeen marked this pull request as ready for review January 11, 2024 20:58
@PureWeen PureWeen requested a review from a team as a code owner January 11, 2024 20:58
@PureWeen PureWeen requested review from Eilon and halter73 and removed request for StephaneDelcroix January 11, 2024 21:08
return Dispatcher.GetForCurrentThread()!;
var dispatch = Dispatcher.GetForCurrentThread();

return dispatch ?? svc.GetRequiredService<ApplicationDispatcher>().AppDispatcher;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the coalescing here, because we don't want to break users that are retrieving the scoped IDispatcher from the root service provider on a background thread. Because we are no longer saturating the IDispatcher on the root provider we need to account for this.

@mattleibow mattleibow marked this pull request as draft January 22, 2024 13:49
@PureWeen PureWeen closed this Jan 23, 2024
@mattleibow mattleibow deleted the scoped_service branch January 25, 2024 21:23
@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-core-hosting Extensions / Hosting / AppBuilder / Startup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable Scope Validation for dependency injection in .NET MAUI during development

3 participants