Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blazor Hybrid/web shared assets via RCL #26331

Merged
merged 20 commits into from Oct 21, 2022
Merged

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Jul 6, 2022

Fixes #26186
Addresses #26848

Internal Review Topic

Notes

  • Although the .NET podcasts sample splits out the audio player implementation for its "web" Blazor app, I felt the general principle is that an RCL can provide a single shared interface and implementations for both native and web when the interface is the same. Dan, your example didn't demo that aspect. I'm 馃憘 if you want to handle it differently (i.e., move the interface and implementation for the "web" to the "web" project).
  • I avoid the word "agnostic" because idk how well that will translate. I went with "independent" instead.
  • This focuses the specific examples on .NET MAUI Blazor but includes NOTEs for WinForms and WPF and uses "Blazor Hybrid" language where the remark generally applies to Hybrid apps.
  • I tested with a .NET MAUI Blazor and Blazor Server app. However, I didn't test with WinForms/WPF, so I recommend either eyeball'in 馃憖 my remarks very carefully or signaling me to go ahead and take some additional time to test the scenarios here in WinForms and WPF projects.
  • This focuses on Blazor Server in the examples for "web" but indicates to readers that the concepts also apply to Blazor WebAssembly. Again, I didn't test Blazor WASM, so check me carefully on those remarks ... or else I can test all of the scenarios with WASM to see if I went wrong anywhere.
  • This focuses on a project ref/example of a RCL class lib project but tells readers that all of the concepts apply equally to publishing the RCL as a NuGet package. If you have any additional guidance on that scenario, I'm 馃憘 for it and will add it to the topic. Alternatively, we can strike all of the NuGet package lingo.
  • Do you want a dotnet/blazor-samples repo sample of this scenario in spite of its forward maintenance costs馃挵? If so, is my test app that mirrors the guidance in the PR (.NET MAUI Blazor + Blazor Server) what you'd like to showcase?

@guardrex guardrex marked this pull request as ready for review July 7, 2022 11:52
Copy link
Member

@danroth27 danroth27 left a comment

Choose a reason for hiding this comment

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

Thanks for pulling this content together, @guardrex! My main feedback is about making the content more generic and less specific to the project templates. Let me know if anything is unclear.

Let's hold off on adding an official sample for this. We may end up needing to support this scenario better in templates, which would supersede and sample.

aspnetcore/blazor/hybrid/class-libraries.md Show resolved Hide resolved
aspnetcore/blazor/hybrid/class-libraries.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/class-libraries.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/class-libraries.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/class-libraries.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/class-libraries.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/class-libraries.md Outdated Show resolved Hide resolved
aspnetcore/blazor/hybrid/class-libraries.md Outdated Show resolved Hide resolved
@guardrex
Copy link
Collaborator Author

@danroth27 ... As of this (Tuesday) morning, I've made updates to address your feedback except for the last point, which will take a little more time. I'll either have that done this afternoon or on Wednesday. It's probably best if you wait until I get the last bit done because I'd also like to edit the whole topic again. I'll ping back when ready ... probably Wednesday morning.

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 19, 2022

@danroth27 ... Perhaps best if you DO look at the very last commit, which only contains the 2nd abstraction example ...

  • First, is this in general what you're looking for? It's just the stream-of-consciousness first draft at this point, but I think you can steer me a different way if this isn't composing well right now.
  • Second ... if WASM and Hybrid share an implementation, it doesn't fit neatly into the default/OOB "Platforms" folder vis-a-vis the .NET MAUI Blazor project template when SxS with the Blazor Server implementation in the proposed "Web" folder. See what I mean? Blazor WASM is web, too. However, if it were pulled out of the Platforms folder, what folder name would be used to hold it? I think you'll need to figure out what the folder names (namespaces) should be for these implementations.

.... and no need to sweat any little nits (spelling, phrasing, etc.). I'll clean that stuff up tomorrow morning. I really did just type this straight in and ping u. There's no polish on this yet.

As for the rest ... the commit prior to the last ... probably best to not look at that yet. I need to 馃挙馃泴馃槾 on it one night and apply a round of updates to clean up and improve it.


When code must differ for across hosting models, abstract the code as interfaces and inject the service implementations into each project.

### Example 1
Copy link
Member

Choose a reason for hiding this comment

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

This first example shows how to deal with multiple target platforms. I think it might be simpler to start with the weather forecast service example, which shows how to deal with different hosting models, and then show the more complicated example of dealing with multiple target platforms.

I believe the Platforms folder convention is also specific to .NET MAUI class libraries that have UseMaui set to true in the project. I don't think we support setting UseMaui to true in an RCL. So you'd need to move the platform specific service implementations into a separate .NET MAUI class library.

This still leaves the question of how to package and distribute the Blazor components with the platform specific implementations. I'm not sure what we should recommend for that. @Eilon @javiercn Thoughts?

Copy link
Collaborator Author

@guardrex guardrex Jul 20, 2022

Choose a reason for hiding this comment

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

start with the weather forecast service example

Done! 馃憤

The rest of your remarks are cryptic to me (e.g., I created the folders in the RCL manually), so I'll wait for more feedback on the exact actions to take.

Also, I performed general updates. The whole PR is ready for another look 馃憖.

Copy link
Member

Choose a reason for hiding this comment

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

I talked this over with @SteveSandersonMS and we agreed to hold off on diving into the complexity of dealing with platform specific logic for now. At this point I think we should remove the 2nd example and just have the first weather forecast example.I would also like to change the weather forecast example to recommend that the IWeatherForecastService implementations go into the corresponding Blazor apps instead of in the RCL.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updates made. ... I held the 2nd example in place in case it's used later (or it's the basis of something used later).

@guardrex guardrex closed this Jul 20, 2022
@guardrex guardrex reopened this Jul 20, 2022
Copy link
Member

@Eilon Eilon left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@guardrex guardrex self-assigned this Sep 29, 2022
Copy link
Member

@danroth27 danroth27 left a comment

Choose a reason for hiding this comment

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

A couple of minor nits, but otherwise this looks good to me :shipit:.

Co-authored-by: Daniel Roth <daroth@microsoft.com>
@guardrex guardrex merged commit d5d4bc5 into main Oct 21, 2022
@guardrex guardrex deleted the guardrex/blazor-hybrid-rcl branch October 21, 2022 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document how to share components for web and native clients with Blazor Hybrid
5 participants