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

Remove ItemCollection from TagHelperDescriptorProviderContext #10720

Merged
merged 8 commits into from
Aug 12, 2024

Commits on Aug 8, 2024

  1. Configuration menu
    Copy the full SHA
    53ec3ab View commit details
    Browse the repository at this point in the history
  2. Make Compilation a TagHelperDescriptorProviderContext property

    Every single `TagHelperDescriptorProviderContext` created in Razor compiler or tooling code adds a valid compilation. So, adding the compilation to the `ItemsCollection` is pure overhead and no `ITagHelperDescriptorProvider` needs to check it for null.
    
    I removed a single test that verified the behavior if a compilation was never set. Now that a compilation is required, this test is no longer needed.
    DustinCampbell committed Aug 8, 2024
    Configuration menu
    Copy the full SHA
    adff92a View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    ec5cd16 View commit details
    Browse the repository at this point in the history
  4. Remove TagHelperDescriptorProviderContext.Items property

    This can be safely removed now that there are no more references.
    DustinCampbell committed Aug 8, 2024
    Configuration menu
    Copy the full SHA
    45625bc View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    9476634 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    eae0d53 View commit details
    Browse the repository at this point in the history
  7. Clean up all ITagHelperDescriptorProviders a bit (and found a bug!)

    - Enable nullability
    - Mark all as sealed
    - Use `Lazy<T>` for singleton `ITagHelperDescriptorProviders`
    - Introduce `TagHelperDescriptorProviderBase`
    - Inherit from `TagHelperDescriptorProviderBase`
    - Remove unnecessary properties
    - Remove unnecessary property setters
    
    In addition, I spotted a bug in `EventHandlerTagDescriptorProvider` while addressing nullability warnings. The buggy code is right here:
    
    https://github.com/dotnet/razor/blob/fb84ae5d9bb8132972c2c23cf209721161f81deb/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/EventHandlerTagHelperDescriptorProvider.cs#L70-L76
    
    When reading the constructor arguments of an `EventHandlerAttribute` it appears that we swap the last two arguments of the 4-argument variant. The constructor is defined like so:
    
    ```C#
    EventHandlerAttribute(string attributeName, Type eventArgsType, bool enableStopPropagation, bool enablePreventDefault);
    ```
    
    Unfortunately, the compiler reads the third parameter as `enablePreventDefaeult` and the fourth parameter as `enableStopPropagation`.
    
    This has been broken since support for the 4-argument constructor variant was added in dotnet@7635bba. Fixing this bug is tracked by dotnet#10497.
    DustinCampbell committed Aug 8, 2024
    Configuration menu
    Copy the full SHA
    5fbfc0d View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    9f6c7b6 View commit details
    Browse the repository at this point in the history