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

FUSE - Component attributes are written as strings #10403

Open
ryzngard opened this issue May 23, 2024 · 4 comments
Open

FUSE - Component attributes are written as strings #10403

ryzngard opened this issue May 23, 2024 · 4 comments
Labels
area-compiler Umbrella for all compiler issues New Feature: Fuse
Milestone

Comments

@ryzngard
Copy link
Contributor

Design time writes out the actual property reference, which causes gotodef to work when mapping. GoToDef fails in fuse for these cases. @davidwengier it looks like in single server we opted to ask for Roslyn to answer these but prior we would handle ourselves? Maybe you have some more insight.

Potential solution: Can the tooling side be smarter here? Can we make a fully qualified name and ask for roslyn to find the definition locations and find a mapping for those?

Fuse

            __builder.OpenComponent<global::BlazorServer.Shared.SurveyPrompt>(5);
            __builder.AddComponentParameter(6, "Title", "How is Blazor working for you?");
            __builder.CloseComponent();

Without Fuse

      __builder.AddAttribute(-1, "ChildContent", (global::Microsoft.AspNetCore.Components.RenderFragment)((__builder2) => {
      }
      ));
      #pragma warning disable BL0005
      ((global::BlazorServer.Shared.SurveyPrompt)default).
#nullable restore
#line 9 "S:\BlazorServer\Pages\Index.razor"
              Title

#line default
#line hidden
#nullable disable
       = default;
      #pragma warning restore BL0005
#nullable restore
#line 9 "S:\BlazorServer\Pages\Index.razor"
@davidwengier
Copy link
Contributor

This was a change in #7974 to enable better tooling for component attributes. Go to Def, Find All Refs, Rename etc. It allows Roslyn to "see" that a component attribute is semantically a property reference.

My preference here would be to maintain the design time behaviour in some way (just look at the number of issues that PR fixes). Potentially replacing the string with a nameof to the fully qualified property? I did originally do nameof in that PR but changed it due to feedback, but maybe it could still be made to work.

Alternatively, we could just generate some extra code for IDE support (maybe a method in a file scoped class that is never called?). I suspect there might be more instances where tooling wants different code than runtime needs.

@ryzngard
Copy link
Contributor Author

I see, the main concern is that nameof is not a reserved word in C#.

Alternatively, we could just generate some extra code for IDE support (maybe a method in a file scoped class that is never called?). I suspect there might be more instances where tooling wants different code than runtime needs.

This might be a good solution in the long run. The question here would be perf oriented since a lot of the runtime code, afaik, is geared towards minimizing edits for interactive rendering.

@davidwengier
Copy link
Contributor

is geared towards minimizing edits for interactive rendering

I'm not sure I understand what this means, but in any case the code generation I'm talking about would have no runtime impact as far as I'm aware. For example, in order for the generate method code action to be available, we could simply emit into the generated code:

	private void ___NeverCalled1() { }

	private void __NeverCalled2() { }

Unless the runtime is using reflection to poke through the generated classes, I wouldn't imagine it would matter.

@phil-allen-msft phil-allen-msft added this to the 17.11 P2 milestone May 23, 2024
@ryzngard
Copy link
Contributor Author

ryzngard commented May 23, 2024

is geared towards minimizing edits for interactive rendering

I'm not sure I understand what this means, but in any case the code generation I'm talking about would have no runtime impact as far as I'm aware. For example, in order for the generate method code action to be available, we could simply emit into the generated code:

	private void ___NeverCalled1() { }

	private void __NeverCalled2() { }

Unless the runtime is using reflection to poke through the generated classes, I wouldn't imagine it would matter.

I was overthinking it. In my mind it was doing something clever like replacing the code with

__builder.AddComponentParameter(6, SomeThing(b => b.Title, "Title"), "How is Blazor working for you?");

string Something(Func<...> _, string) s) => s;

Which in hindsight and not mid-meeting clarity is definitely wrong :)

@ryzngard ryzngard added the area-compiler Umbrella for all compiler issues label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-compiler Umbrella for all compiler issues New Feature: Fuse
Projects
None yet
Development

No branches or pull requests

3 participants