Skip to content

#51611 - Extended Blazor server and webassembly options to match web format. #54649

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

Closed
wants to merge 9 commits into from

Conversation

khamzakhamzakhamza
Copy link

#51611 - Extended Blazor server and webassembly options to match web format.

Extends options consumed by the Blazor.start() method in blazor.server.js and blazor.webassembly.js to match how they work in blazor.web.js and in the documentation.

Description

I followed the design proposed by @MackinnonBuck and implemented the suggested changes. I have tested changes locally and everything works as expected.

I couldn't find any unit or integration tests for the Boot classes and felt hesitant to create new ones. However, I am happy to do so if someone from the team decides they are needed.

@khamzakhamzakhamza khamzakhamzakhamza requested a review from a team as a code owner March 20, 2024 18:12
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Mar 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 20, 2024
@khamzakhamzakhamza
Copy link
Author

@khamzakhamzakhamza please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Mar 30, 2024
khamzakhamzakhamza and others added 4 commits March 30, 2024 10:44
Update .NET SDK to version 9.0.100-preview.5.24253.17.

---
updated-dependencies:
- dependency-name: Microsoft.NET.Sdk
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…00-preview.5.24253.17

Update .NET SDK to 9.0.100-preview.5.24253.17
@khamzakhamzakhamza khamzakhamzakhamza requested review from wtgodbe and a team as code owners May 9, 2024 22:30
github-actions bot and others added 4 commits June 10, 2024 12:27
Update .NET SDK to version 9.0.100-preview.6.24309.2.

---
updated-dependencies:
- dependency-name: Microsoft.NET.Sdk
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…100-preview.6.24309.2

Update .NET SDK to 9.0.100-preview.6.24309.2
@MackinnonBuck
Copy link
Member

/azp run

@dotnet-policy-service dotnet-policy-service bot removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Dec 12, 2024
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

Sorry to revoke the approval, but we'll likely need tests to ensure that this doesn't break in the future. E2E tests are probably the easiest way to test this (rather than trying to test the Boot.*.ts code directly).

@MackinnonBuck
Copy link
Member

Related to #56758. We'd only want to take one of these changes, as they do the same thing.

Copy link
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Dec 20, 2024
@lewing lewing requested a review from maraf January 22, 2025 03:07
@lewing lewing removed their assignment Jan 22, 2025
@maraf
Copy link
Member

maraf commented Feb 3, 2025

@khamzakhamzakhamza Thank you for your contribution and sorry for a late reply.

We need some automated tests validating that options are correctly propagated. The easiest way to achieve it is through E2E test. There are tests that contains some startup options and we need to modify them to run once with wasm/server options as they do now and second with options matching web format. For WebAssembly such test candidate might be https://github.com/dotnet/aspnetcore/blob/main/src/Components/test/E2ETest/Tests/WebAssemblyConfigureRuntimeTest.cs#L33 that is running https://github.com/dotnet/aspnetcore/blob/main/src/Components/test/testassets/BasicTestApp/wwwroot/index.html#L66-L70. For Server we probably need to create a new test

@guardrex
Copy link
Contributor

guardrex commented Feb 7, 2025

@maraf ... It looks like this won't make Preview 1. I have the docs PR (dotnet/AspNetCore.Docs #34660) ready to merge whenever this reaches a preview release.

@javiercn javiercn added the pr: pending author input For automation. Specifically separate from Needs: Author Feedback label Feb 12, 2025
Copy link
Contributor

Hi @khamzakhamzakhamza.
It seems you haven't touched this PR for the last two weeks. To avoid accumulating old PRs, we're marking it as stale. As a result, it will be closed if no further activity occurs within 4 days of this comment. You can learn more about our Issue Management Policies here.

@dotnet-policy-service dotnet-policy-service bot added the stale Indicates a stale issue. These issues will be closed automatically soon. label Feb 22, 2025
@guardrex
Copy link
Contributor

guardrex commented Feb 26, 2025

@maraf @javiercn ... DAH! 😄 ... I have the docs PR (dotnet/AspNetCore.Docs #34660) set up to react to this. I'm 🤞🍀 that this can make .NET 10 so that my work doesn't go to waste. It was a pain to set up due to the complex article versioning required.

@lewing lewing reopened this Feb 26, 2025
@guardrex
Copy link
Contributor

guardrex commented Mar 3, 2025

@lewing ... This closed again. Let me know if it's going to be dropped ... I'll close the docs PR if that happens.

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 community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun pr: pending author input For automation. Specifically separate from Needs: Author Feedback stale Indicates a stale issue. These issues will be closed automatically soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants