Blazor RC1 coverage #36053
Blazor RC1 coverage #36053
Conversation
guardrex
force-pushed
the
guardrex/blazor-rc1
branch
from
26c419a to
c738aba
Compare
guardrex
requested review from
danroth27,
javiercn,
MackinnonBuck,
ilonatommy and
oroztocil
| ## Failure to preserve types used by a published app | ||
|
|
||
| Trimming may have detrimental effects for a published app leading to runtime errors. In apps that use [reflection](/dotnet/csharp/advanced-topics/reflection-and-attributes/), the IL Trimmer often can't determine the required types for runtime reflection and trims them away or trims away parameter names from methods. This can happen with complex framework types used for JS interop, JSON serialization/deserialization, and other operations. | ||
| Trimming may have detrimental effects for a published app leading to runtime errors, even in spite of setting the [`<PublishTrimmed>` property](#configuration) to `false` in the project file. In apps that use [reflection](/dotnet/csharp/advanced-topics/reflection-and-attributes/), the IL Trimmer often can't determine the required types for runtime reflection and trims them away or trims away parameter names from methods. This can happen with complex framework types used for JS interop, JSON serialization/deserialization, and other operations. |
There was a problem hiding this comment.
even in spite of setting the
<PublishTrimmed>property tofalse
This doesn't sound right. Setting PublishTrimmed to false should disable trimming for the entire app. Does this not work? If it doesn't, I think it's a bug that we should fix, not document.
There was a problem hiding this comment.
I'll double check this in the morning and report back.
There was a problem hiding this comment.
There was a problem hiding this comment.
@gaurdrex Can you create an issue for this in the dotnet/aspnetcore repo and share your project with us?
There was a problem hiding this comment.
@danroth27 ... Isn't this the same as (or at least has the elements of) dotnet/aspnetcore#52947? It's just not KeyValuePair ... it's a Tuple ... but same situation/error. I thought @javiercn was well aware of this, and I placed the content that he requested to cover it. I just want to make sure I'm opening an issue that @javiercn won't close saying that it can never be fixed outside of the approaches provided ... in order: use a custom type (any .NET), Dynamic Dependency (.NET 5+), Root Descriptor (.NET 10+), <_ExtraTrimmerArgs> (.NET 8 workaround).
There was a problem hiding this comment.
... and @danroth27 ... are we holding this PR past the RC1 release if @javiercn doesn't respond by then?
There was a problem hiding this comment.
Isn't this the same as (or at least has the elements of) dotnet/aspnetcore#52947? It's just not KeyValuePair ... it's a Tuple ... but same situation/error. I thought @javiercn was well aware of this, and I placed the content that he requested to cover it.
Right, that's why I thought @javiercn might want to review this content.
are we holding this PR past the RC1 release if @javiercn doesn't respond by then?
Nope, no need to block on this. The content looks fine to me.
There was a problem hiding this comment.
I just asked because you were calling for his review. I'll react with a patch PR later if he has any changes.
... and I thought Javier would just re-open the existing PU issue on that subject, but I'm still 👂 if you/he want a new issue on ConstructorContainsNullParameterNames.
There was a problem hiding this comment.
We did KeyValuePair because people use that.
If get reports for other types, we might choose to add those too. The cost of preserving a type is small compared to the benefit of not having to force people into doing this type of stuff if enough people use the type.
|
|
||
| For more information, see [Trimming options (.NET documentation)](/dotnet/core/deploying/trimming/trimming-options#trimming-granularity). | ||
|
|
||
| ## Failure to preserve types used by a published app |
There was a problem hiding this comment.
@javiercn Can you review the recommendations made in this section for dealing with type preservation for JS interop related scenarios?
There was a problem hiding this comment.
@javiercn Is there more you want to add here for handling persistent component state with enhanced nav?
|
@danroth27 @javiercn @oroztocil ... Updates made ... and there were quite a few, so a new look 👁️ is a good idea. I still need ...
If possible, I'd like to merge this PR no later than noon EST (9am PST). It won't go live until the Merge-to-Live PR goes in at release time. |
Addresses #34437
Fixes #35897
Fixes #35972
Fixes #35360
Fixes #36070
Notes
I searched for other items to cover for RC1, but I didn't find anything else.
Sorry that this time I couldn't set up one commit per feature. There were feature changes across commits this time.
In
aspnetcore/mvc/models/validation.mdfor the Built-in attributes section, some common attributes are listed. Instead of the arbitrary list and having to version to get new ones shown, I recommend that we just cross-link the API Browser ...I'm holding on to the old table locally. Note that if you want the table instead of the cross-link that I'm going to need to copy the entire table (version the whole thing) to get the new
[SkipValidation]attribute listed.Internal previews