Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: dotnet/razor
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: d5cfe11b00e279b96344440e4d9b403994b376de
Choose a base ref
...
head repository: dotnet/razor
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 3920563359e73a7b8d3c8d7a1bba61bbf1c9efe5
Choose a head ref
  • 13 commits
  • 46 files changed
  • 3 contributors

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 7635bba. Fixing this bug is tracked by #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

Commits on Aug 9, 2024

  1. Unskip rename tests

    davidwengier committed Aug 9, 2024
    Configuration menu
    Copy the full SHA
    4441e1d View commit details
    Browse the repository at this point in the history
  2. Try to fix rename tests

    davidwengier committed Aug 9, 2024
    Configuration menu
    Copy the full SHA
    ed8dcee View commit details
    Browse the repository at this point in the history
  3. Move to central package pinning (#10716)

    * Move to central package pinning
    
    This should make it much easier for us to respond to CG alerts in the
    future. All that will need to be done is add an entry in
    Directory.Packages.props and it will automatically impact all consumers
    of it.
    
    Consider this example in Roslyn for how to respond to a CG issue
    
    dotnet/roslyn#74653
    jaredpar authored Aug 9, 2024
    Configuration menu
    Copy the full SHA
    cd1f82b View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    9044af2 View commit details
    Browse the repository at this point in the history

Commits on Aug 12, 2024

  1. Remove ItemCollection from TagHelperDescriptorProviderContext (#10720)

    Every `TagHelperDescriptorProviderContext` creates an `ItemCollection`
    to hold onto, at most, two objects: a `Compilation` and a target
    `ISymbol`. This is enormously wasteful because an `ItemCollection`
    internally creates a `ConcurrentDictionary<object, object>`. So, I've
    changed `TagHelperDescriptorProviderContext` to just hold onto a
    compilation and a target symbol and avoid the `ItemCollection`
    altogether.
    
    I recommend reviewing commit-by-commit.
    
    Also, I bumped into a long standing compiler bug in
    `EventHandlerTagHelperDescriptorProvider` that was recently filed by a
    customer: #10497. I opted to stay
    on target and not fix this issue, but I made it painfully obvious where
    the fix would go. 😄
    DustinCampbell authored Aug 12, 2024
    Configuration menu
    Copy the full SHA
    3920563 View commit details
    Browse the repository at this point in the history
Loading