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

Automatically disable compression in Razor SDK projects #93

Closed
robertmclaws opened this issue Aug 26, 2024 · 5 comments · Fixed by #94
Closed

Automatically disable compression in Razor SDK projects #93

robertmclaws opened this issue Aug 26, 2024 · 5 comments · Fixed by #94

Comments

@robertmclaws
Copy link
Contributor

The default setting for the WebCompiler is to automatically compress any output resources.

A problem has arisen where these artifacts are causing issues in .NET 9 projects because WebCompiler is not creating the proper manifests for the compressed and uncompressed files, which breaks the .NET 9 compiler.

Would be great if the Target could detect if it's running in a Razor SDK project and automatically disable compression so the built-in Blazor compilation system can handle that instead.

Microsoft CoPilot suggested the following target to help you make that determination, and I've verified that it works even if RazorLangVersion is not specified in the .csproj file (which means the Razor SDK sets it automatically):

<Target Name="CheckIfRazorProject" AfterTargets="Build">
  <PropertyGroup>
    <IsRazorProject Condition="'$(RazorLangVersion)' != ''">true</IsRazorProject>
  </PropertyGroup>
  <Message Text="Is this a Razor project? $(IsRazorProject)" Importance="high" />
</Target>

HTH!

@stefanloerwald
Copy link
Member

Thanks @robertmclaws for raising this and connecting the dots. Unfortunately, this is only something we can improve in the documentation. This project itself doesn't detect the environment it's running in.

We could consider changing the default value for a major version release, at the same time as the release of net9.0. WDYT?

@robertmclaws
Copy link
Contributor Author

Great to meet you and thanks for the quick response!

Seems like there would be an easy way to change the docs, especially here:

<Target Name="CompileStaticAssets" AfterTargets="CoreCompile" >
    <PropertyGroup>
        <!-- Compression should be disabled by default if we're in a Razor Class Library -->
        <WebCompilerCompression Condition="'$(RazorLangVersion)' != ''">disabled</WebCompilerCompression>
        <WebCompilerCompression Condition="'$(RazorLangVersion)' == ''">enabled</WebCompilerCompression>
    </PropertyGroup>

    <!-- Test if WebCompiler is installed -->
    <Exec Command="webcompiler -h" ContinueOnError="true" StandardOutputImportance="low" StandardErrorImportance="low" LogStandardErrorAsError="false" IgnoreExitCode="true">
        <Output TaskParameter="ExitCode" PropertyName="ErrorCode" />
    </Exec>

    <!-- Execute the WebCompiler task if the test was successful -->
    <Exec Command="webcompiler -r wwwroot --zip ${WebCompilerCompression}" StandardOutputImportance="high" StandardErrorImportance="high" Condition="'$(ErrorCode)' == '0'" />
</Target>

That would combine everything into a single target, set the proper compression option, detect if it's installed, and executes the command with the compression option set.

In this case, there would be no need to change the defaults.

HTH!

@stefanloerwald
Copy link
Member

Wow, thanks for writing this, that looks like exactly what is needed here.

Would you mind creating a pull request for this? I can approve those much sooner than I can write one myself.

Regarding default value: neither that, nor changing the documentation will fix any project affected by this, as it requires changes there. But I'd guess the chance that someone updates packages is higher than them actively changing the build integration. So it could be useful for more people implicitly.

@robertmclaws
Copy link
Contributor Author

Done. Important to note that those changes SHOULD work, but I might have missed something.

Regarding changing the defaults, that's up to you... but it seems like a good idea.

Thanks!

@stefanloerwald
Copy link
Member

Thanks for that. I'll consider changing the default at another time, so I'll keep this issue open for the interim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@robertmclaws @stefanloerwald and others