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

Use Roslyn interceptors feature in config binder generator #89322

Closed
Tracked by #79527
layomia opened this issue Jul 21, 2023 · 6 comments · Fixed by #90340
Closed
Tracked by #79527

Use Roslyn interceptors feature in config binder generator #89322

layomia opened this issue Jul 21, 2023 · 6 comments · Fixed by #90340

Comments

@layomia
Copy link
Contributor

layomia commented Jul 21, 2023

ref: @captainsafia's work on the request delegate source generator - dotnet/aspnetcore#48817.

cc @tarekgh @eerhardt @ericstj.

@ghost
Copy link

ghost commented Jul 21, 2023

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

ref: @captainsafia's work on the request delegate source generator - dotnet/aspnetcore#48817.

cc @tarekgh @eerhardt @ericstj.

Author: layomia
Assignees: layomia
Labels:

blocking-release, area-Extensions-Configuration

Milestone: 8.0.0

@layomia
Copy link
Contributor Author

layomia commented Jul 21, 2023

Can take this opportunity to fix #86363.

@layomia
Copy link
Contributor Author

layomia commented Aug 9, 2023

@tarekgh @ericstj @eiriktsarpalis looks like the interceptors feature requires <Features>$(Features);InterceptorsPreview</Features> in consuming projects. While the feature is in preview, it seems we should update the SDK to set it if EnableConfigurationBindingGenerator is set. Otherwise users would need to set two properties to use the generator.

The Web SDK already sets the feature property when binding & RDG generates are enabled, so the concern is about regular SDK scenarios.

@tarekgh
Copy link
Member

tarekgh commented Aug 9, 2023

@layomia does the interceptor feature will be released the same way? I mean users have to opt-in to get it. Or is this temporary for preview and will change when it gets released?

@layomia
Copy link
Contributor Author

layomia commented Aug 10, 2023

Yes I believe interceptors will be shipped as a preview feature in GA. cc @captainsafia.

@ericstj
Copy link
Member

ericstj commented Aug 10, 2023

We can do the same thing as RDG here. Just add us to this condition:
https://github.com/dotnet/sdk/blob/b3b5a2800d4de997572068332bf222e2e8f808d5/src/WebSdk/ProjectSystem/Targets/Microsoft.NET.Sdk.Web.ProjectSystem.targets#L62-L65

We could do the same in our nuget package targets:
https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Configuration.Binder/src/buildTransitive/Microsoft.Extensions.Configuration.Binder.targets

Might be good to have a safeguard in the generator just in case someone manages to invoke it without setting that property. I suspect RDG needs the same as well (eg Web class library that get's referenced by a normal app or class library).

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 10, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 19, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants