Skip to content

[Fixes #12588][Blazor] Move Blazor to use Static Web Assets #18409

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 37 commits into from
Feb 19, 2020
Merged

[Fixes #12588][Blazor] Move Blazor to use Static Web Assets #18409

merged 37 commits into from
Feb 19, 2020

Conversation

javiercn
Copy link
Member

  • Plugs-in Blazor wasm through the static web assets infrastructure.
  • Avoids the need for a custom Blazor.config file.
  • Removes broken auto-rebuild and debug support.
  • Removes unnecessary server-side Blazor helpers.

The main benefits of this change are:

  • Client-side blazor apps are treated as static web assets, so they benefit from all the static web assets infrastructure.
    • Things like publish single file just works.
    • Running inside docker container just works.
  • No need to reference the client-side app assembly from the server-side app:
    • Allows for the client and server app to have incompatible dependencies.
    • Reduces the size of the server app output as the client assemblies are only copied once.
  • Highly reduces the number of customization to host blazor apps inside ASP.NET Core
    • They just become static files and there's no need to have additional runtime components to handle development vs production, or running a published app.

This is a preview2 change,

There are a couple of loose ends here that I'll tackle before preview2 but not in this PR. I'm putting the PR out so that we can get it reviewed early and merged as the changes are "medium-sized" and I don't want them to get stale. The two loose ends are:

  • Startup API
    • Right now startup uses the static files middleware and MapFallbackToFile.
    • I expect these two things to get subsumed into a MapClientSideBlazor helper.
      • I want to tackle that piece separately.
    • For right now, I've put the static files configuration inside a service collection helper, but that will go away before preview2 ships.
  • Debugging support
    • Right now the current logic unconditionally registers pdb extensions to be served from the file provider.
    • I don't think this is a big deal and we can make it configurable based on the host environment or a configuration setting.

This can go in as soon as we've shipped preview1 and the changes we made in the 3.1 patch branches have made it to the blazor-wasm branch.

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jan 17, 2020
@javiercn javiercn requested a review from pranavkm January 17, 2020 10:01
pranavkm
pranavkm reviewed Jan 18, 2020
View reviewed changes
@pranavkm pranavkm added this to the blazor-wasm-3.2-preview2 milestone Jan 18, 2020
@javiercn javiercn mentioned this pull request Jan 21, 2020
Closed
@SteveSandersonMS
Copy link
Member

@javiercn Since this is for preview 2, is it OK if I leave it a week before reviewing this? If that's going to derail your work then please let me know and I'll try to find space to dig into it sooner.

SteveSandersonMS
SteveSandersonMS reviewed Feb 10, 2020
View reviewed changes
SteveSandersonMS
SteveSandersonMS reviewed Feb 10, 2020
View reviewed changes
@javiercn javiercn force-pushed the javiercn/blazor-swa-blazor-wasm branch from 3f0e24b to 9f8dd5c Compare February 13, 2020 20:57
@javiercn javiercn force-pushed the javiercn/blazor-swa-blazor-wasm branch 4 times, most recently from 3fb8167 to ed2afbd Compare February 18, 2020 01:38
@javiercn javiercn force-pushed the javiercn/blazor-swa-blazor-wasm branch 2 times, most recently from dbe72d7 to 799fb16 Compare February 18, 2020 13:43
javiercn
javiercn commented Feb 18, 2020
View reviewed changes
Copy link
Member Author

@javiercn javiercn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Ended up the standalone publish output to the wwwroot folder as discussed here.
  • Plugged Copy and Publish through Static Web Assets.
  • Updated tests accordingly.
  • Updated copying the pdbs to use BlazorDebuggingEnabled

@javiercn
[Blazor] Move Blazor to use Static Web Assets
0ad8047 
* Plugs-in Blazor wasm through the static web assets infrastructure.
* Avoids the need for a custom Blazor.config file.
* Removes broken auto-rebuild and debug support.
* Removes unnecessary server-side Blazor helpers.
@javiercn
Cleanups
2802298
@javiercn
Cleanups
1c375d5
@javiercn
Fix templates
b719d1b
@javiercn
Revert SWA cleanup
6a2a076
@javiercn
Test manifest entries
cd70ea9
@javiercn
Fix tests on non windows platforms
12ac0b3
@javiercn
Fix additional separator
4216051
@javiercn javiercn force-pushed the javiercn/blazor-swa-blazor-wasm branch from 24120f3 to 9f73e71 Compare February 18, 2020 19:47
@javiercn javiercn force-pushed the javiercn/blazor-swa-blazor-wasm branch from 97f5608 to 14b7f0e Compare February 18, 2020 21:27
pranavkm
pranavkm reviewed Feb 19, 2020
View reviewed changes
@javiercn javiercn force-pushed the javiercn/blazor-swa-blazor-wasm branch from 841bcf1 to a271758 Compare February 19, 2020 08:31
SteveSandersonMS
SteveSandersonMS reviewed Feb 19, 2020
View reviewed changes
src/Components/test/E2ETest/Tests/BootResourceCachingTest.cs
@@ -57,7 +57,7 @@ public void CachesResourcesAfterFirstLoad()
var subsequentResourcesRequested = GetAndClearRequestedPaths();
Assert.NotEmpty(initialResourcesRequested.Where(path => path.EndsWith("/blazor.boot.json")));
Assert.Empty(subsequentResourcesRequested.Where(path => path.EndsWith("/dotnet.wasm")));
Assert.NotEmpty(subsequentResourcesRequested.Where(path => path.EndsWith(".js")));
Assert.Empty(subsequentResourcesRequested.Where(path => path.EndsWith(".js")));
Copy link
Member

@SteveSandersonMS SteveSandersonMS Feb 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Why did this change? I would expect it always to request blazor.webassembly.js and dotnet.js regardless of the caching, so I don't get how this can be right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I didn't see it being requested.

Ah, it's likely due to the removal of the no-cache header we put in blazor.webassembly.js. We can chat a bit more about this, but it shouldn't be necessary for us to include the no-cache header, the browser will do the right thing.

The browser already does the right thing. We can talk about this later if you want.

Copy link
Member

Choose a reason for hiding this comment

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

That doesn’t sound valid. We need the browser to request this since its content may have changed. Or if not, how does the browser do the right thing? I would expect the no-cache header is needed.

Let’s not consider this resolved just yet :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed offline, I'll send a follow-up PR to resolve this.

SteveSandersonMS
SteveSandersonMS approved these changes Feb 19, 2020
View reviewed changes
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Looks like time to 🚢 !

I do have one actual correctness question and have posted a couple of suggestions about comment typos, but once those are resolved this is looking great to me.

@javiercn javiercn merged commit 0e155fd into blazor-wasm Feb 19, 2020
@javiercn javiercn deleted the javiercn/blazor-swa-blazor-wasm branch February 19, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pranavkm pranavkm pranavkm left review comments

@SteveSandersonMS SteveSandersonMS SteveSandersonMS approved these changes

@ryanbrandenburg ryanbrandenburg Awaiting requested review from ryanbrandenburg

Assignees
No one assigned
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Milestone
blazor-wasm-3.2-preview2
Development

Successfully merging this pull request may close these issues.
3 participants
@javiercn @SteveSandersonMS @pranavkm