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

Feature: Build static assets conditionally for Backoffice and Login #17527

Merged
merged 15 commits into from
Nov 19, 2024

Conversation

iOvergaard
Copy link
Contributor

@iOvergaard iOvergaard commented Nov 13, 2024

Description

This PR replaces #15055 following the release and subsequent merge of the new Backoffice (Bellissima) files.

This change aims to use the MSBuild functionalities described in Build client web assets for your Razor Class Library, which the original PR also adapted, to build the Backoffice and Login targets conditionally if any of the source files changed. This ensures that developers can rely on having the latest client assets built when running the executable such as Umbraco.Web.UI, which depends on the StaticAssets project.

The change will ensure that:

  • npm ci will be run when package.json or package-lock.json change
  • npm run build will be run if any of the source files differ from the last time you built the Backoffice or Login

Other changes:

  • The "preconditions" have been removed, which checked on file paths, because we are now storing the build output to either backoffice.complete.txt or login.complete.txt in the intermediary output folder. These files get deleted when you clean the solution ensuring, that the build targets will rebuild the assets.
  • The "clean" conditions have also been removed because the normal "clean" target already removes the intermediary output folder, where we store our *.complete.txt output files ensuring, that the build targets run after a clean the next time you build.

Note: I have not moved the client files into the StaticAssets project as the original PR proposed to do. We want to keep these separate web projects for ease of use and contribution.

How to test

  1. Run the Umbraco.Web.UI executable on a clean project and check that the installer, login, and ultimately backoffice clients are shown and usable in the browser.
  2. Make a change to a *.ts source file somewhere and check that the relevant client assets are rebuilt when you run the Umbraco.Web.UI executable.
  3. Run dotnet clean on the staticassets project and/or the whole solution, then run the Umbraco.Web.UI executable and verify that everything is rebuilt.

@iOvergaard
Copy link
Contributor Author

@ronaldbarendse maybe this is something you'd like to take a look at seeing that you proposed the original PR #15055?

@leekelleher leekelleher self-assigned this Nov 13, 2024
@iOvergaard
Copy link
Contributor Author

Thanks, @ronaldbarendse. Your changes seem to work. Good idea with the DefineStaticWebAssets.

Comment on lines +15 to +20
<!-- Added project references as the sample API in umbraco-extension will fail -->
<ItemGroup>
<Content Include="..\src\Umbraco.Web.UI\Program.cs">
<Link>UmbracoProject\Program.cs</Link>
<PackagePath>UmbracoProject</PackagePath>
</Content>
<Content Include="UmbracoProject\**" Exclude="bin;obj" />
<Content Include="UmbracoExtension\**" Exclude="bin;obj" />
<Content Include="UmbracoDockerCompose\**" Exclude="bin;obj" />
<Content Include="..\src\Umbraco.Web.UI\Views\Partials\blocklist\**">
<Link>UmbracoProject\Views\Partials\blocklist\%(RecursiveDir)%(Filename)%(Extension)</Link>
<PackagePath>UmbracoProject\Views\Partials\blocklist</PackagePath>
</Content>
<Content Include="..\src\Umbraco.Web.UI\Views\Partials\blockgrid\**">
<Link>UmbracoProject\Views\Partials\blockgrid\%(RecursiveDir)%(Filename)%(Extension)</Link>
<PackagePath>UmbracoProject\Views\Partials\blockgrid</PackagePath>
</Content>
<Content Include="..\src\Umbraco.Web.UI\Views\_ViewImports.cshtml">
<Link>UmbracoProject\Views\_ViewImports.cshtml</Link>
<PackagePath>UmbracoProject\Views</PackagePath>
</Content>
<ProjectReference Include="..\src\Umbraco.Cms.Api.Common\Umbraco.Cms.Api.Common.csproj" />
<ProjectReference Include="..\src\Umbraco.Cms.Api.Management\Umbraco.Cms.Api.Management.csproj" />
<ProjectReference Include="..\src\Umbraco.Web.Common\Umbraco.Web.Common.csproj" />
<ProjectReference Include="..\src\Umbraco.Web.Website\Umbraco.Web.Website.csproj" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're now exposing these as public dependencies, but a template project doesn't actually use/need dependencies to run (as long as the generated code does)... I'll create another PR to fix this 😉

Comment on lines 8 to +9
<IsPackable>true</IsPackable>
<EnablePackageValidation>false</EnablePackageValidation>
<NoWarn>NU5100</NoWarn>
<EnablePackageValidation>$(BaseEnablePackageValidation)</EnablePackageValidation>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any projects that is distributed as a package should have package validation enabled (use the setting value defined in Directory.Build.props). The Umbraco.Cms.Tests.Integration package contains public helper classes, which shouldn't contain breaking changes within the the current major!

However, all integration tests should be excluded by this, so maybe we can make all tests internal and not publicly expose them in the shipped package? Again, this is something to fix in a separate PR...

</Target>
<ItemGroup>
<BackofficeAssetsInputs Include="$(BackofficeProjectDirectory)package.json;$(BackofficeProjectDirectory)package-lock.json;$(BackofficeProjectDirectory)src\**" Exclude="$(DefaultItemExcludes)" />
<Content Remove="$(BackofficeAssetsPath)\**" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the generated files from the project, so they don't show up and also to avoid duplicate errors due to them otherwise being added to the StaticWebAsset item group by default (the custom DefineBackofficeAssets task adds them already)...

@ronaldbarendse
Copy link
Contributor

The DefineStaticWebAssets is needed for dynamically generated files, because they otherwise won't be included if the files don't exist yet when the build starts. This can be tested by cleaning the git repository and using a single command that does all restore, build and pack operations in one go:

git clean -dxf
dotnet pack -c Release -o build.out

I've cleaned also up the project files as well, so all WarningsAsErrors items are nicely formatted and top-level dependencies that are added to update vulnerable versions are grouped together as well. Finally, package validation is enabled again on the Umbraco.Cms.Tests.Integration package and the removed TemplateServiceTests.Deleting_Master_Template_Also_Deletes_Children() test is suppressed, as that's not 'officially' part of the public helpers.

Copy link
Member

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it out from the CLI and all worked as expected! 🚀

@iOvergaard iOvergaard merged commit 07b7c26 into v15/dev Nov 19, 2024
16 of 18 checks passed
@iOvergaard iOvergaard deleted the v15/feature/staticassets-msbuild-conditions branch November 19, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants