Skip to content
This repository has been archived by the owner on Jun 2, 2024. It is now read-only.

Migrate off of sassc #224

Open
1 task done
nullishamy opened this issue May 24, 2024 · 6 comments
Open
1 task done

Migrate off of sassc #224

nullishamy opened this issue May 24, 2024 · 6 comments
Labels
meta priority:low This is a low priority incident and can be pushed back.

Comments

@nullishamy
Copy link
Contributor

Is there an existing issue outlining your problem?

  • I have searched the existing issues and they do not solve my problem.

Describe the issue.

Sassc is now EOL, so we should move off of it, and use Dart Sass.
From the sassc repo:

Warning: LibSass and SassC are deprecated. While it will continue to receive maintenance releases indefinitely, there are no plans to add additional features or compatibility with any new CSS or Sass features. Projects that still use it should move onto Dart Sass.

@isabelroses
Copy link
Member

Been messing around, seems like its just changing the command name. Maybe the flags. Needs a bit more testing.

@nullishamy nullishamy added the priority:low This is a low priority incident and can be pushed back. label May 24, 2024
@nullishamy
Copy link
Contributor Author

Marking as low priority, not a blocker for v1.0.0. @isabelroses trialed dart sass and it seems to have odd breakages and significant performance regressions. Additionally, sassc will get maintenance releases for now, and we have no issues with it.

We can investigate these issues at some point and report them to dart sass and/or fix them locally if applicable.

@isabelroses
Copy link
Member

isabelroses commented May 24, 2024

Below is a list of all my findings between the differences. I heavily suggest we stay with sassc for now.

  • The diff between rc3 and dart-sass, includes an incredible line spam making the gtk-3.0/gtk-dark.css file balloon from a line count of 8538 to 157511. And this is not just for this file, this also exists for gtk-3.0/gtk.css. No such similar problems exist in other files.

  • Previously rgb values that could be matched to a name like maroon or blue, were converted to such, now they are not. (this is not a problem really)

  • The diff, is heavily polluted by missing newlines, even using the previous flags converted to dart-sass flags.

  • There are also some other miscellaneous errors with numbers just becoming 0. e.g.
    image

  • Significantly worse performance. See Bad performance sass compared to sassc sass/dart-sass#1161 for more.
    Please read the below carefully, we are going from milliseconds to seconds.

    # sassc
    Benchmark 1: python3 build.py frappe -a blue --dest ./dist
    Time (mean ± σ):     129.3 ms ±  22.6 ms    [User: 63.9 ms, System: 13.7 ms]
    Range (min … max):    93.1 ms … 195.6 ms    22 runs
    
    # dart-sass
    Benchmark 2: python3 build.py frappe -a blue --dest ./dist
    Time (mean ± σ):     24.714 s ±  2.286 s    [User: 24.502 s, System: 0.506 s]
    Range (min … max):   18.475 s … 26.011 s    10 runs
    
For those who want to test, you can apply this patch:

From e0ddcb0c2ba96c47a9c18cfd461372e957fd1459 Mon Sep 17 00:00:00 2001
From: isabel <isabel@isabelroses.com>
Date: Fri, 24 May 2024 13:35:21 +0100
Subject: [PATCH] refactor: sassc -> dart-sass

---
 build.py  | 26 +++++++++++++-------------
 shell.nix |  2 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/build.py b/build.py
index fe0a321..d65c41d 100755
--- a/build.py
+++ b/build.py
@@ -9,7 +9,7 @@ from catppuccin.models import Flavor, Color

 THIS_DIR = os.path.dirname(os.path.realpath(__file__))
 SRC_DIR = f"{THIS_DIR}/colloid/src"
-SASSC_OPT = ["-M", "-t", "expanded"]
+SASS_OPT = ["-q", "--no-source-map"]

 logger = logging.getLogger("catppuccin-gtk")
 logger.setLevel(logging.DEBUG)
@@ -95,8 +95,8 @@ def build(ctx: BuildContext):
     )
     subprocess.check_call(
         [
-            "sassc",
-            *SASSC_OPT,
+            "sass",
+            *SASS_OPT,
             f"{SRC_DIR}/main/gnome-shell/gnome-shell{ctx.apply_suffix(DARK_LIGHT)}.scss",
             f"{output_dir}/gnome-shell/gnome-shell.css",
         ]
@@ -105,16 +105,16 @@ def build(ctx: BuildContext):
     os.makedirs(f"{output_dir}/gtk-3.0", exist_ok=True)
     subprocess.check_call(
         [
-            "sassc",
-            *SASSC_OPT,
+            "sass",
+            *SASS_OPT,
             f"{SRC_DIR}/main/gtk-3.0/gtk{ctx.apply_suffix(DARK_LIGHT)}.scss",
             f"{output_dir}/gtk-3.0/gtk.css",
         ]
     )
     subprocess.check_call(
         [
-            "sassc",
-            *SASSC_OPT,
+            "sass",
+            *SASS_OPT,
             # NOTE: This uses 'Dark' for the source, but 'dark' for the destination. This is intentional. Do !!NOT!! change it without consultation
             f"{SRC_DIR}/main/gtk-3.0/gtk-Dark.scss",
             f"{output_dir}/gtk-3.0/gtk-dark.css",
@@ -124,16 +124,16 @@ def build(ctx: BuildContext):
     os.makedirs(f"{output_dir}/gtk-4.0", exist_ok=True)
     subprocess.check_call(
         [
-            "sassc",
-            *SASSC_OPT,
+            "sass",
+            *SASS_OPT,
             f"{SRC_DIR}/main/gtk-4.0/gtk{ctx.apply_suffix(DARK_LIGHT)}.scss",
             f"{output_dir}/gtk-4.0/gtk.css",
         ]
     )
     subprocess.check_call(
         [
-            "sassc",
-            *SASSC_OPT,
+            "sass",
+            *SASS_OPT,
             # NOTE: This uses 'Dark' for the source, but 'dark' for the destination. This is intentional. Do !!NOT!! change it without consultation
             f"{SRC_DIR}/main/gtk-4.0/gtk-Dark.scss",
             f"{output_dir}/gtk-4.0/gtk-dark.css",
@@ -143,8 +143,8 @@ def build(ctx: BuildContext):
     os.makedirs(f"{output_dir}/cinnamon", exist_ok=True)
     subprocess.check_call(
         [
-            "sassc",
-            *SASSC_OPT,
+            "sass",
+            *SASS_OPT,
             f"{SRC_DIR}/main/cinnamon/cinnamon{ctx.apply_suffix(DARK_LIGHT)}.scss",
             f"{output_dir}/cinnamon/cinnamon.css",
         ]
diff --git a/shell.nix b/shell.nix
index 7305ad6..f82a658 100644
--- a/shell.nix
+++ b/shell.nix
@@ -6,7 +6,7 @@ pkgs.mkShell {
   buildInputs = with pkgs; [
     python311
     python311Packages.catppuccin
-    sassc
+    dart-sass
     inkscape
     optipng
   ];
--
2.44.1

@nex3
Copy link

nex3 commented May 29, 2024

Staying on LibSass isn't going to be viable in the long term. It's only receiving strictly necessary security updates, and even those very slowly. It will not support new CSS or Sass features, and it won't receive functionality bug fixes.

I'm struggling to figure out how to run exactly the same inputs you're running, but from compiling Colloid-gtk-theme/src/main/gtk-3.0/gtk.scss directly with Dart Sass I've come up with some information.

  • The diff between rc3 and dart-sass, includes an incredible line spam making the gtk-3.0/gtk-dark.css file balloon from a line count of 8538 to 157511. And this is not just for this file, this also exists for gtk-3.0/gtk.css. No such similar problems exist in other files.

This code uses @extend heavily. @extend was never implemented precisely to spec in LibSass, so it's not surprising that there are some differences. I think the line bloat is coming primarily from the fact that Dart Sass gives up on trimming redundant selectors once a selector list is more than 100 selectors long; removing that exception brings the output size from 157.5k to 7.4k lines. That heuristic was in place to avoid quadratic behavior, but it's actually faster without it, so I may just remove it from Dart Sass entirely.

  • Previously rgb values that could be matched to a name like maroon or blue, were converted to such, now they are not. (this is not a problem really)

This is intentional. In expanded mode, Dart Sass preserves the original format in which colors were specified whenever possible.

  • The diff, is heavily polluted by missing newlines, even using the previous flags converted to dart-sass flags.

There's no guarantee that different implementations will produce identical outputs. (In particular, Dart Sass doesn't add newlines between adjacent at-rules, or between style rules nested within the same parent rule.)

  • There are also some other miscellaneous errors with numbers just becoming 0. e.g.
    image

I can't find or reproduce this. Can you provide a minimal reproduction?

This I can reproduce, although for me it's upwards of 2x faster without the heuristic I mentioned above. It looks like the vast majority of time (at least for gtk-3.0) is being spent resolving extensions. Again, LibSass cut some corners there which allowed it to be faster at the expense of correctness so some of this may be unavoidable. In the future we may switch @extend to start using :is() rather than manually expanding selector lists, which would substantially reduce both the compilation time and the output size, at the expense of slightly different specificity and lower compatibility with old browsers.

@nullishamy
Copy link
Contributor Author

Yeah, we will have to move eventually, but currently that is not viable (primarily due to performance). Would appreciate if you could give us an update if/when dartsass implements the patches you described which would help performance. It's currently not possible for us to refactor the theme away from @extend at this time.

@nex3
Copy link

nex3 commented May 29, 2024

sass/dart-sass#2255 is out to remove the heuristic, which should land and be released in the text few days. That's likely to be the best we can offer in terms of structural performance improvements, though; there may be a few minor optimizations we can do, but looking at a profiler the majority of time is spent assembling the extensions. Unless we can push the trimming logic earlier in the process to avoid generating and them removing a bunch of selectors—not impossible but a substantial engineering effort that we don't have bandwidth for—it's likely that you're going to be looking at tens of seconds regardless.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
meta priority:low This is a low priority incident and can be pushed back.
Projects
None yet
Development

No branches or pull requests

3 participants