chore(dotnet): allow nullable channels #36114
Conversation
Test results for "tests 1"11 flaky39282 passed, 808 skipped Merge workflow run. |
dgozman
left a comment
dgozman
left a comment
There was a problem hiding this comment.
Could you please post an example of the code before and after, with a link to the repo source?
Something like microsoft/playwright-dotnet@f374bd0? |
| let suffix = '' | ||
| if (!['bool', 'int', 'System.Text.Json.JsonElement'].includes(inner.ts)) | ||
| suffix = ' = null!;' | ||
| ts.push(`${indent}public ${inner.ts}${nullableSuffix(inner)} ${toTitleCase(name)} { get; set; }${suffix}`); |
There was a problem hiding this comment.
It seems like this forces everything to be non-null, instead of allowing nullable channels? Could you please explain better what we are trying to achieve?
There was a problem hiding this comment.
When enabling nullability in the repository .NET complains that these classes we generate can be created with null values which is correct. In order to avoid that there are three ways:
a) constructor which forces to initialise these values - this doesn't apply to JSON serialises, since these classes are never constructed via us, they are always constructed via the underlying json library. So thats not an option.
b) use the required attribute - this won't work since we have netstandard2.0 but it was introduced in netstandard2.1. So thats not an option.
c) use = null!; - its the verbose way of skipping this null check, internally it was null anyways. In the future we could change it to the required attribute once we move to a more recent TFM.
2 participants
This patch updates the .NET channel generator to support nullable reference types. Although objects are constructed via
System.Text.Jsondeserialization, the compiler still requires non-nullable reference properties to be initialized. To avoid warnings when nullable is enabled, this change adds= null!;for reference types (excludingbool,int, andJsonElement).microsoft/playwright-dotnet#3163