adoconnection / RazorEngineCore

.NET6 Razor Template Engine
MIT License
565 stars 84 forks source link

Add CancellationToken overloads to Compile methods #134

Closed daviddotcs closed 8 months ago

daviddotcs commented 11 months ago

Since CSharpCompilation.Emit and CSharpSyntaxTree.Parse support cancellation, it'd be good to be able to forward cancellation tokens down to those potentially long-running methods. Ideally they'd have been optional parameters added to each of the compile methods, however that would be a breaking change to the API so I've added them as overloads.

daviddotcs commented 10 months ago

I inadvertently made a bigger breaking change (where any overrides of RazorEngine.CreateAndCompileToStream(string, RazorEngineCompilationOptions) would no longer be called) by trying to avoid breaking changes to the API, so I've reverted back to proposing the Compile and CompileAsync methods have an optional CancellationToken parameter added, and the CreateAndCompileToStream has a non-optional CancellationToken parameter added.

adoconnection commented 10 months ago

Hi 👋 thanks for pull request. Do we really need cancellation token in non async methods?

daviddotcs commented 10 months ago

Hi. Yes, I believe both the asynchronous and synchronous methods would benefit from being cancellable. The CSharpCompilation.Emit and CSharpSyntaxTree.Parse methods are both synchronous and support cancellation, for example.

I did a brief bit of searching on the matter and didn't find too much, but in https://learn.microsoft.com/en-us/dotnet/standard/threading/cancellation-in-managed-threads it makes the following statement which suggests that supporting cancellation tokens in synchronous methods is valid:

Starting with .NET Framework 4, .NET uses a unified model for cooperative cancellation of asynchronous or long-running synchronous operations.

adoconnection commented 8 months ago

released in 2023.11.1 🚀 https://www.nuget.org/packages/RazorEngineCore/