NuGet / NuGetGallery

NuGet Gallery is a package repository that powers https://www.nuget.org. Use this repo for reporting NuGet.org issues.
https://www.nuget.org/
Apache License 2.0
1.52k stars 643 forks source link

[Dark Theme] Fix failing functional tests related to minification #9935

Closed martinrrm closed 1 month ago

martinrrm commented 2 months ago

Functional tests related to minification started to fail with this new feature. What is happening is that CSS minification isn't able to understand the CSS variables because we are using an old package to handle bundle/minification (System.Web.Optimization from 2014).

Found:    Minification failed
      In value: /* Minification failed. Returning unminified contents.
      (296,15): run-time error CSS1039: Token not allowed after unary operator: '-brandForegroundLinkRest'
      (301,15): run-time error CSS1039: Token not allowed after unary operator: '-brandForegroundLinkHover'
      (306,27): run-time error CSS1039: Token not allowed after unary operator: '-neutralStrokeFocus2Rest'
      (533,15): run-time error CSS1039: Token not allowed after unary operator: '-neutralForeground3Rest'
      (622,26): run-time error CSS1039: Token not allowed after unary operator: '-neutralBackground5Rest'
      (1582,15): run-time error CSS1039: Token not allowed after unary operator: '-neutralForeground3Rest'
      (1819,15): run-time error CSS1039: Token not allowed after unary operator: '-neutralForeground2Rest'
      (1821,33): run-time error CSS1039: Token not allowed after unary operator: '-neutralStroke2Rest'
      (1838,15): run-time error CSS1039: Token not allowed after unary operator: '-neutralForeground1Rest'
      (1859,26): run-time error CSS1039: Token not allowed af
      Stack Trace:
        D:\a\_work\1\s\tests\NuGetGallery.FunctionalTests\StaticAssets\StaticAssetsTests.cs(145,0): at NuGetGallery.FunctionalTests.StaticAssets.StaticAssetsTests.<NoBundleFailsMinification>d__14.MoveNext()
        --- End of stack trace from previous location where exception was thrown ---
           at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
           at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
        --- End of stack trace from previous location where exception was thrown ---
           at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
           at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
        --- End of stack trace from previous location where exception was thrown ---
           at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
           at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

Fix

Grunt also creates minified files every time we use grunt command, with this change I'm no longer adding the files to the site.min.css bundle to avoid minification and instead use the minified file we already have.

Addresses https://github.com/NuGet/NuGetGallery/issues/9936

martinrrm commented 2 months ago

Functional tests are failing when I added a new test, not sure why, this is the error. I'm continuing the investigation here.

image

erdembayar commented 1 month ago

@martinrrm There's merge conflict needs to be addressed

martinrrm commented 1 month ago

@joelverhagen @agr Finally fixed tests https://dev.azure.com/nuget/NuGetBuild/_build/results?buildId=100646&view=results. Code has changed a little bit but it's the same solution.

When bundling bootstrap.css to styles.min.css it is minifying the file but the tool that does it doesn't understand about CSS variables because is from 2014, and it no longer maintained.

This solution is to add bootstrap.min.css to the source files and use it, we were already creating it when running grunt, so it's not adding a lot of new code and avoid re-minifying it.

We only need to reference it in one file like we are doing with styles.min.css

erdembayar commented 1 month ago

When bundling bootstrap.css to styles.min.css it is minifying the file but the tool that does it doesn't understand about CSS variables because is from 2014, and it no longer maintained.

Which tool are you referring here?

martinrrm commented 1 month ago

@erdembayar So I've looked in the documentation and found that is using this package https://learn.microsoft.com/en-us/previous-versions/aspnet/hh195125(v=vs.110) which I believe it's been outdated since 2013.

There is some documentation on how to minify other type of files (less, scss and sass which are other preprocessors) here https://learn.microsoft.com/en-us/aspnet/mvc/overview/performance/bundling-and-minification#less-coffeescript-scss-sass-bundling, they recommend installing another package and parsing the file with it. Which technically is what I'm doing here but with grunt.