-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Updated the Blazor Hybrid Solution template to work better with the dotnet cli or IDEs that rely on it. #25997
base: main
Are you sure you want to change the base?
Conversation
…otnet cli or IDEs that rely on it.
Hey there @jbw716! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that this is what you expect. The condition says to not create a sln in an IDE, because the IDE is already creating a sln - that is the only way for it to have multiple projects loaded at once.
{ | ||
"condition": "(HostIdentifier != \"dotnetcli\" && HostIdentifier != \"dotnetcli-preview\")", | ||
"path": "MauiApp.1.Shared/Pages/Home.razor" | ||
}, | ||
{ | ||
"condition": "(HostIdentifier == \"dotnetcli\" || HostIdentifier == \"dotnetcli-preview\")", | ||
"path": "MauiApp.1.sln" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just telling VS that it should open these files. If you do this, the CLI will now have an error saying it can't open files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally fair here. I am still relatively new to templates but trying to get this one to work for non-VS users.
{ | ||
"condition": "(HostIdentifier != \"dotnetcli\" && HostIdentifier != \"dotnetcli-preview\")", | ||
"exclude": [ | ||
"*.sln" | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to exclude the sln creation in VS because the IDE will have issues as it creates its own sln already.
Here is the ASP.NET templates doing just this: https://github.com/dotnet/aspnetcore/blob/main/src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/.template.config/template.json#L116-L119
VSCode uses the CLI, so they will get the sln. Rider should be relying on the sln in the template because they will have their own as well. I would strongly suggest opening an issue on Rider's side if they are not creating a sln for a multi-project template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this not, then, check if VS and if so, exclude the sln? The template at the link you provided also does not provide a sln file, however, since it is only a single project template, it's not nearly as big of an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open an issue in the IDE that is not creating a sln as that is an IDE bug, not a behaviour that should be happening.
I updated the PR with a new suggestion and better clarification around the issue. |
Pinging @vijayrkn and @phenning to understand if this is logically the right change. @mattleibow is indeed correct with the intention of the logic: VS already creates SLNs automatically, so if we create one via the template then the world is less happy. But then this same exact condition is copied in probably dozens of templates across multiple repos. I've never seen a check for Is there an example of a scenario that doesn't work? |
Description of Change
Updated the Blazor Hybrid Solution template to better support using the dotnet cli or IDEs that rely on it, like VSCode and Rider.
Issues Fixed
Excluding the .sln file doesn't make sense, since other IDEs can use the .sln file as well. If it's not needed, it can be deleted, much like other templates do. Not having it in the first place makes life significantly more difficult for users of this template on non Visual Studio environments.
Fixes #
Updated the template to remove the conditionals that exclude the .sln file for dotnetcli.