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

Add Nullable annotation support in MudBlazor. #6535

Open
75 of 91 tasks
ScarletKuro opened this issue Mar 26, 2023 · 1 comment
Open
75 of 91 tasks

Add Nullable annotation support in MudBlazor. #6535

ScarletKuro opened this issue Mar 26, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request epic

Comments

@ScarletKuro
Copy link
Member

ScarletKuro commented Mar 26, 2023

Is your feature request related to a problem?

Update the codebase with nullable reference types to improve null diagnostic warnings, more info here.

Not only end user will benefit from this, but also the codebase quality will increase.
NullReferenceException would be much rare. For example, currently the DataGrid suffers from this issue.
Nowadays, the <Nullable>enable</Nullable> is the default when you create a new projects.

List of projects that has the nullable enabled on the project level

List of components that should be annotated
Base:

Misc:

Components:

I will volunteer and do 1 component at time when I get the time.

Describe the solution you'd like

Plan:

  1. Pick a component.
  2. Add preprocessor directive - #nullable enable to component files.
  3. Properly annotate the code.
  4. Submit the PR.

Repeat until no components left to annotate and enable enable on project leave and remove the preprocessor directive.
Rule: Don't make multiple components at once since it will be hard to review.

Technical aspects

This doesn't force the user to use nullable, nothing will change for people who doesn't use Nullable in their application, it will work as it is now..
Technically, this is not API change. In fact, the compiler adds additional metedata: NullableAttribute and NullableContextAttribute.
Example:

public string? Name { get; set; }

Transform to

[Nullable(2)]
[CompilerGenerated]
private string __BackingField;

[Nullable(2)]
public string Name
{
  [NullableContext(2), CompilerGenerated] get
  {
	return this.__BackingField;
  }
  [NullableContext(2), CompilerGenerated] [param: In] set
  {
	this.__BackingField = value;
  }
}

This says that the property Name can be null. It's just added some metadata around.
But people who have the nullable enabled will notice the difference.
Let's say I'm MudBlazor user and call Name.ToString() the Roslyn will warn me saying that you should check for null since the Name can be null and I get the NullReferenceException.

Now the opposite, if there is no ? on Name means that it's guaranteed by the library to always have a value and I can safely call Name.ToString() without checking for null.
If I as a MudBlazor user try to assign null to Name Roslyn will also warn me that this is not desired.

Have you seen this feature anywhere else?

BlazorStrap, fluentui-blazor(MS) - surprisingly, many UI Blazor frameworks don't support it.
But if we look more widely, then most popular Nuget packages do support it, including the .NET BCL.

Pull Request

  • I would like to do a Pull Request

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ScarletKuro ScarletKuro added enhancement New feature or request triage labels Mar 26, 2023
@github-actions
Copy link

👋 Thanks for wanting to do a PR @ScarletKuro !
We suggest that you wait for an answer from @MudBlazor/contribution-team @MudBlazor/core-team . Otherwise we can not guarantee that your PR will be merged. As the library grows, we have to be very strict what PRs we can accept.

@henon henon removed the triage label Apr 15, 2024
@ScarletKuro ScarletKuro unpinned this issue May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request epic
Projects
None yet
Development

No branches or pull requests

2 participants