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

[Blazor] Constructor injection #53915

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Conversation

javiercn
Copy link
Member

@javiercn javiercn commented Feb 9, 2024

Fixes #18088

Reverts 87f870d

@javiercn javiercn requested review from wtgodbe and a team as code owners February 9, 2024 11:30
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Feb 9, 2024
@javiercn javiercn force-pushed the javiercn/constructor-injection branch from 7e1f4a4 to a404424 Compare February 9, 2024 12:04
@javiercn javiercn removed request for wtgodbe and a team February 9, 2024 12:31
@SteveSandersonMS
Copy link
Member

Can you document how we know this doesn't reintroduce the problems we had with constructor injection before? #40521

@javiercn
Copy link
Member Author

javiercn commented Feb 9, 2024

@SteveSandersonMS #18088 (comment)

@javiercn
Copy link
Member Author

javiercn commented Feb 9, 2024

@SteveSandersonMS I suspect is dotnet/runtime@a48e73a

It's essentially using new APIs.

I haven't done size testing myself, but given we are early in the release, I think we can make this go in, and observe if we find any regression in our regular runs. Are you ok with that?

@javiercn javiercn force-pushed the javiercn/constructor-injection branch from a404424 to ca2bbf1 Compare February 9, 2024 14:53
@SteveSandersonMS
Copy link
Member

I haven't done size testing myself, but given we are early in the release, I think we can make this go in, and observe if we find any regression in our regular runs. Are you ok with that?

It's less for us to keep track of if you just do a one-off manual publish locally to see the impact on final sizes, otherwise if some regression appears two weeks from now, we have to go through a whole detective process to trace it back to the cause.

@javiercn
Copy link
Member Author

javiercn commented Feb 9, 2024

It's less for us to keep track of if you just do a one-off manual publish locally to see the impact on final sizes, otherwise if some regression appears two weeks from now, we have to go through a whole detective process to trace it back to the cause.

I did measure the change for the Blazor hosted app in release configuration between main and this branch and the size is the same.

@javiercn javiercn merged commit 22ec418 into main Feb 9, 2024
26 checks passed
@javiercn javiercn deleted the javiercn/constructor-injection branch February 9, 2024 18:39
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview2 milestone Feb 9, 2024
onurmicoogullari pushed a commit to onurmicoogullari/aspnetcore that referenced this pull request Feb 14, 2024
* Adds support for constructor injection in components.
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.

Constructor injection for Blazor components
3 participants