rungwiroon / BlazorGoogleMaps

Blazor interop for GoogleMap library
MIT License
309 stars 99 forks source link

Link KeyProvider to map object scope instead of application level static #326

Closed TheAtomicOption closed 1 month ago

TheAtomicOption commented 2 months ago

I'm attempting to write up a PR, so this issue is for tracking/discussion purposes:

The DependencyInjectionExtensions.AddBlazorGoogleMaps() method introduced in 4.2 doesn't actually add a service or support building a KeyProvider service. Or at least in a way I can see how to accomplish. Maybe I don't understand how to use Func for that purpose? (@valentas-deltamina if you have any input on why this property is Func<string> instead of just string and or how that might be used for something more complex than an anonymous function that returns a const string, that'd be helpful.)

AddBlazorGoogleMaps() then assigns the BlazorGoogleMapsOptions object to a static variable on the extension class. This is fine if you have a synchronous function returning one api key for your entire app, but seems to disallow async key lookup and/or dynamically using different API keys at different times (for example on an app that is a platform for multiple sites that need to use their own keys).

Lastly the BlazoGoogleMapsOptions purpose seems to somewhat overlap conceptually with Maps.MapOptions which can be defined more dynamically.


The current goals of the PR I'm working on are:


Would appreciate any input/suggestions on the best approach/architecture to reach the above goals. Especially if I'm missing some obvious solution that doesn't even require making a PR to do key lookup through an async lookup at a point in execution where the URL of the current HTTP request or blazor page/route is available as a parameter to the lookup.

valentasm1 commented 2 months ago

There was no much thinking. I really didnt like idea to keep api key in some variable. Still dint like it that it sync I agree that it better have some interface for key provider and transient service. It would solve platform for multiple sites that need to use their own keys Regarding overlap with MapOptions. Moving options to map options need more setup by user but maybe increase map loading speed since in some maps you dont need all additional libraries. More i think more i agree with moving to MapOptions mainly due perfomance impact. Still i think key registration should be in DependencyInjectionExtensions with some Interface There is bug which i left for next iterations :). If you have two maps in same page you make two call to blazorGoogleMaps.objectManager.initMap which i wrong. Lets start little by little. You could create Interface with logic for key provider. After that merge options to MapOptions.

valentasm1 commented 1 month ago

I think we could close it.