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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React to Mono WebAssembly changes for 3.1 preview3 #16631

Conversation

SteveSandersonMS
Copy link
Member

This is necessary when we receive the changes in dotnet/blazor#1923

Until then, this will fail on build.

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Oct 29, 2019
@SteveSandersonMS SteveSandersonMS added this to the 3.1.0-preview3 milestone Oct 29, 2019
/// Gets or sets the default value of the 'credentials' option on outbound HTTP requests.
/// Defaults to <see cref="FetchCredentialsOption.SameOrigin"/>.
/// </summary>
public static FetchCredentialsOption DefaultCredentials
Copy link
Member Author

Choose a reason for hiding this comment

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

I know this is not going to survive API review eventually, but it's the closest thing to what we already have. I don't want to design a new thing just as part of reacting to an external change.

Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

The biggest change in this PR is the removal of our own HttpClient handler in favor of one provided by Mono, is that the case?

I don't remember stuff about the MSBuild piece, but that might change in the future, so I think this is good.

Thanks for taking care of this!

@SteveSandersonMS
Copy link
Member Author

The biggest change in this PR is the removal of our own HttpClient handler in favor of one provided by Mono, is that the case?

Yes. Mono has removed the ability to set a custom default HttpMessageHandler, which we were doing before. The default is now always the one in Mono's WebAssembly.Net.Http assembly. Overall this is a good thing because now other things like HttpClientFactory should start working as expected.

The main complication is that we don't expose any reference to Microsoft.AspNetCore.Blazor.Mono, as that is used during build only. Therefore there's no design-time reference to WebAssembly.Net.Http either, hence having to do all the reflection to set the default credentials options. I think it's fine since it's purely an internal implementation detail. Eventually we might want Microsoft.AspNetCore.Blazor to take a package dependency on WebAssembly.Net.Http but we can't do that today in any reasonable way because WebAssembly.Net.Http doesn't get published anywhere as a package.

@campersau
Copy link
Contributor

So this fixes some of the HTTP streaming issues like #5586, #13329 ?
But we would have to enable streaming manually via reflection because of this mono/mono#17328 ?

@pranavkm
Copy link
Contributor

Yes. Mono has removed the ability to set a custom default HttpMessageHandler, which we were doing before. The default is now always the one in Mono's WebAssembly.Net.Http assembly. Overall this is a good thing because now other things like HttpClientFactory should start working as expected.

FYI @anurse \ @BrennanConroy

@BrennanConroy
Copy link
Member

The default is now always the one in Mono's WebAssembly.Net.Http assembly

Are you saying that new HttpClientHandler(); will just work now and use the one Mono provides?

@SteveSandersonMS
Copy link
Member Author

Are you saying that new HttpClientHandler(); will just work now and use the one Mono provides?

I had thought that would be the case, but we just spoke with the Mono team today and apparently it is not. There is currently still a separate wasm-specific HTTP handler; the change is that it's provided by Mono now and not by Blazor.

We have asked for this design to be reconsidered because we do want things like new HttpClientHandler to just work, but there isn't an ETA on that yet.

@BrennanConroy You might also be interested to know that we're not far off having proper ClientWebSocket support in Mono WebAssembly, so the .NET SignalR client should be able to work on wasm soon. We're not there yet because currently you have to new up a completely different wasm-specific alternative to ClientWebSocket right now, but that's another thing we've asked to be changed.

@SteveSandersonMS
Copy link
Member Author

@Pilchie Need approval for preview 3

@Pilchie
Copy link
Member

Pilchie commented Oct 30, 2019

Approved for 3.1.0-Preview3.

@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/react-to-mono-wasm-changes-for-3.1-preview3 branch from f7a2a34 to 0860243 Compare October 31, 2019 09:54
@SteveSandersonMS
Copy link
Member Author

The changes here have been merged as part of #16685

@SteveSandersonMS SteveSandersonMS deleted the stevesa/react-to-mono-wasm-changes-for-3.1-preview3 branch October 31, 2019 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants