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

Add CancellationToken overloads to Compile methods #134

Merged
merged 3 commits into from
Nov 17, 2023
Merged

Add CancellationToken overloads to Compile methods #134

merged 3 commits into from
Nov 17, 2023

Conversation

daviddotcs
Copy link
Contributor

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 daviddotcs marked this pull request as draft September 2, 2023 04:02
@daviddotcs
Copy link
Contributor Author

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.

@daviddotcs daviddotcs marked this pull request as ready for review September 2, 2023 04:32
@adoconnection
Copy link
Owner

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

@daviddotcs
Copy link
Contributor Author

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 adoconnection merged commit 3ed53cf into adoconnection:master Nov 17, 2023
1 check failed
@adoconnection
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants