-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Removes npm commands from MSBuild of the CSPROJ for umbraco-extension dotnet new template #20839
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
base: release/17.0
Are you sure you want to change the base?
Removes npm commands from MSBuild of the CSPROJ for umbraco-extension dotnet new template #20839
Conversation
…tension dotnet new template Was agreed by the community package team to remove this, as this DX can cause more issues than actually help users in our opinion
|
@AndyButland & @iOvergaard hopefully this targets the correct branch for you and has the relevant information/detail in the PR for you. |
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.
Pull Request Overview
This PR removes automatic NPM build integration from the umbraco-extension dotnet new template's MSBuild process. The change addresses deployment issues with platforms like Umbraco Cloud (which lack Node.js) and aligns with the expectation that developers using CI/CD pipelines will explicitly manage their npm build steps.
- Removes two MSBuild targets (
RestoreClientandBuildClient) that automatically rannpm iandnpm run build - Shifts responsibility to developers to manually run npm commands or configure them in their CI/CD pipelines
Comments suppressed due to low confidence (1)
templates/UmbracoExtension/Umbraco.Extension.csproj:29
- The
ClientAssetsInputsItemGroup is now unused and should be removed. It was previously referenced only in theInputsattribute of the removedBuildClienttarget. Keeping unused MSBuild items can confuse developers and adds unnecessary configuration.
<ClientAssetsInputs Include="Client\**" Exclude="$(DefaultItemExcludes)" />
|
Looks good @warrenbuckley, thanks. I'm convinced! Think Copilot has a point here doesn't it? Can this be removed too? templates/UmbracoExtension/Umbraco.Extension.csproj:29
|
|
Ah good catch by Copilot - will clean it up 😄 |
…ithub.com/warrenbuckley/Umbraco-CMS into package-team/remove-csproj-from-extension
Overview
Removes npm commands from the MSBuild of the CSPROJ of the umbraco-extension dotnet new template
Why
It was the community package team opinion that this perhaps can cause more issues than actually help users.
When using Umbraco Cloud and adding the
umbraco-extensiontemplate project to an Umbraco Cloud project it would fail to build and deploy on git push due to Umbraco cloud not having node installed and we felt it odd to have explicit MSBuild properties to look for that mention Umbraco cloud in order to disable this, as not every project created with this dotnet new template may never ever get deployed to Umbraco cloud.Another scenario discussed on the call was that you could get into a scenario where to regenerate TypeScript definitions from the website and an OpenAPI spec and run the website would fail to run if you had failing client side build.
It was generally thought that users using CI/CD would be aware to do the commands
npm installandnpm run buildin their pipelines, if they was not directly committing the client-side build output to their codebase.Community Package Team
This was agreed by the community package teams call on 14th November to remove this from the
dotnet newtemplate forumbraco-extension