Refactor views for Nullable Reference Types?

[JasonElkin]

Two of my favourite new features in .NET core are Nullable Reference Types and Razor compilation.

With both switched on, I get a lot of warnings from the Umbraco views that are imported into my project.

I know that these views will work but I think it would be best if these warnings can be fixed or suppressed. We could refactor the views and add in the necessary null conditionals/checks etc. or, simply add #nullable disable to the files in question.

[KevinJump]

i think these might make a good candidate for hacktoberfest ?

the majority of the issues appear to be in the templates folder
https://github.com/umbraco/Umbraco-CMS/tree/5bfab13dc5a268714aad2426a2b68ab5561a6407/src/Umbraco.Web.UI/umbraco/PartialViewMacros/Templates

these files aren't directly used by a site, but they are what you get when you add a new partial macro and pick from a template. a quick look and i think most of them need some basic null checking. eg. loginstatus.cshtml,

has :

        <p>Welcome back <strong>@Context.User.Identity.Name</strong>!</p>

and something like this would fix it: (interestingly this line would never be hit if any of it was null and its checked higher up - i don't know if this is a bug or limitation in the nullable reference code)

        <p>Welcome back <strong>@Context.User?.Identity?.Name</strong>!</p>

[JasonElkin]

Yes, and I have already made a start. These templates are straightforward enough but I think the Grid is a little more opinionated and will constitute some breaking changes. Think I'll just make a PR and see what happens. 🤷‍♂️

[nul800sebastiaan]

@JasonElkin best not put in breaking changes, with non-breaking changes. we might be fine with putting breaking things in v10 if it makes sense (we don't want to break things just for the sake of it), so we could maybe merge to v10, but you wouldn't want non-breaking stuff to also have to wait for 10.

[JasonElkin]

Sure @nul800sebastiaan, I think it'll need to be more than one PR to account for that

I'll make it either way and we can have a discussion off the back of it. It certainly shouldn't cause an errors - quite the opposite, it will fix a number of potential null reference exceptions (after all, that's the whole point of NRTs) but that might mean the the grid no longer throws (or inline's errors) when people would expect. It's more of a change of behaviour than a breaking breaking change.

[JasonElkin]

Changes may not be as controversial as I first thought, have created a PR: #11370