From 6e522e4aad6814fae79bcb46ffe73449394101f8 Mon Sep 17 00:00:00 2001 From: Rafael dos Santos Silva Date: Tue, 7 Feb 2023 12:24:57 -0300 Subject: [PATCH] DEV: Move to Sass compilation to dart-sass (#19910) This PR is a major change to Sass compilation in Discourse. The new version of sass-ruby moves to dart-sass putting we back on the supported version of Sass. It does so while keeping compatibility with the existing method signatures, so minimal change is needed in Discourse for this change. This moves us From: - sassc 2.0.1 (Feb 2019) - libsass 3.5.2 (May 2018) To: - dart-sass 1.58 This update applies the following breaking changes: > > These breaking changes are coming soon or have recently been released: > > [Functions are stricter about which units they allow](https://sass-lang.com/documentation/breaking-changes/function-units) beginning in Dart Sass 1.32.0. > > [Selectors with invalid combinators are invalid](https://sass-lang.com/documentation/breaking-changes/bogus-combinators) beginning in Dart Sass 1.54.0. > > [/ is changing from a division operation to a list separator](https://sass-lang.com/documentation/breaking-changes/slash-div) beginning in Dart Sass 1.33.0. > > [Parsing the special syntax of @-moz-document will be invalid](https://sass-lang.com/documentation/breaking-changes/moz-document) beginning in Dart Sass 1.7.2. > > [Compound selectors could not be extended](https://sass-lang.com/documentation/breaking-changes/extend-compound) in Dart Sass 1.0.0 and Ruby Sass 4.0.0. SCSS files have been migrated automatically using `sass-migrator division app/assets/stylesheets/**/*.scss` --- Gemfile | 7 ++-- Gemfile.lock | 33 ++++++++++++------- app/assets/stylesheets/common/base/group.scss | 4 ++- .../stylesheets/common/base/groups.scss | 4 ++- .../common/base/magnific-popup.scss | 4 ++- .../common/components/user-card.scss | 4 ++- .../stylesheets/common/foundation/math.scss | 22 +++++++------ .../stylesheets/common/foundation/mixins.scss | 4 ++- .../common/foundation/variables.scss | 4 ++- .../common/select-kit/flair-row.scss | 8 +++-- app/models/theme_field.rb | 2 +- lib/stylesheet/compiler.rb | 1 + lib/stylesheet/manager/builder.rb | 2 +- spec/lib/stylesheet/compiler_spec.rb | 2 +- spec/lib/stylesheet/importer_spec.rb | 2 +- spec/lib/stylesheet/manager_spec.rb | 6 ++-- spec/models/theme_field_spec.rb | 2 +- spec/services/email_style_updater_spec.rb | 2 +- 18 files changed, 70 insertions(+), 43 deletions(-) diff --git a/Gemfile b/Gemfile index 62b2dcf4e0..93cb5e09da 100644 --- a/Gemfile +++ b/Gemfile @@ -229,10 +229,9 @@ gem "logstash-event", require: false gem "logstash-logger", require: false gem "logster" -# NOTE: later versions of sassc are causing a segfault, possibly dependent on processer architecture -# and until resolved should be locked at 2.0.1 -gem "sassc", "2.0.1", require: false -gem "sassc-rails" +# These are forks of sassc and sassc-rails with dart-sass support +gem "dartsass-ruby" +gem "dartsass-sprockets" gem "rotp", require: false diff --git a/Gemfile.lock b/Gemfile.lock index bb2378ede8..5400501961 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -121,6 +121,14 @@ GEM crass (1.0.6) css_parser (1.14.0) addressable + dartsass-ruby (3.0.1) + sass-embedded (~> 1.54) + dartsass-sprockets (3.0.0) + dartsass-ruby (~> 3.0) + railties (>= 4.0.0) + sprockets (> 3.0) + sprockets-rails + tilt date (3.3.3) debug_inspector (1.1.0) diff-lcs (1.5.0) @@ -159,6 +167,7 @@ GEM gc_tracer (1.5.1) globalid (1.1.0) activesupport (>= 5.0) + google-protobuf (3.21.12) guess_html_encoding (0.0.11) hana (1.3.7) hashdiff (1.0.1) @@ -429,15 +438,17 @@ GEM sanitize (6.0.1) crass (~> 1.0.2) nokogiri (>= 1.12.0) - sassc (2.0.1) - ffi (~> 1.9) - rake - sassc-rails (2.1.2) - railties (>= 4.0.0) - sassc (>= 2.0) - sprockets (> 3.0) - sprockets-rails - tilt + sass-embedded (1.58.0) + google-protobuf (~> 3.21) + rake (>= 10.0.0) + sass-embedded (1.58.0-aarch64-linux-gnu) + google-protobuf (~> 3.21) + sass-embedded (1.58.0-arm64-darwin) + google-protobuf (~> 3.21) + sass-embedded (1.58.0-x86_64-darwin) + google-protobuf (~> 3.21) + sass-embedded (1.58.0-x86_64-linux-gnu) + google-protobuf (~> 3.21) selenium-webdriver (4.8.0) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) @@ -540,6 +551,8 @@ DEPENDENCIES cose cppjieba_rb css_parser + dartsass-ruby + dartsass-sprockets diffy digest discourse-fonts @@ -627,8 +640,6 @@ DEPENDENCIES ruby-readability rubyzip sanitize - sassc (= 2.0.1) - sassc-rails selenium-webdriver shoulda-matchers sidekiq diff --git a/app/assets/stylesheets/common/base/group.scss b/app/assets/stylesheets/common/base/group.scss index 458933a8d3..5ef4362f9b 100644 --- a/app/assets/stylesheets/common/base/group.scss +++ b/app/assets/stylesheets/common/base/group.scss @@ -2,6 +2,8 @@ // To style group content differently, use the existing classes with a .group parent class. // For example: .group .user-secondary-navigation +@use "sass:math"; + .group-details-container { background: var(--primary-very-low); padding: 20px; @@ -55,7 +57,7 @@ } $size: 50px; - $icon-size: $size / 1.8; + $icon-size: math.div($size, 1.8); .avatar-flair-image { width: $size; diff --git a/app/assets/stylesheets/common/base/groups.scss b/app/assets/stylesheets/common/base/groups.scss index 0fc72d0b99..b05cf1d383 100644 --- a/app/assets/stylesheets/common/base/groups.scss +++ b/app/assets/stylesheets/common/base/groups.scss @@ -1,3 +1,5 @@ +@use "sass:math"; + .groups-header { display: flex; flex-wrap: wrap; @@ -114,7 +116,7 @@ } $size: 40px; - $icon-size: $size / 1.8; + $icon-size: math.div($size, 1.8); .group-avatar-flair { display: inline-block; diff --git a/app/assets/stylesheets/common/base/magnific-popup.scss b/app/assets/stylesheets/common/base/magnific-popup.scss index bd23ba1e89..bc83d8dc67 100644 --- a/app/assets/stylesheets/common/base/magnific-popup.scss +++ b/app/assets/stylesheets/common/base/magnific-popup.scss @@ -27,6 +27,8 @@ // 1. Default Settings //////////////////////// +@use "sass:math"; + $overlay-color: #0b0b0b !default; $overlay-opacity: 0.8 !default; $shadow: 0 0 8px rgba(0, 0, 0, 0.6) !default; // shadow on image or iframe @@ -46,7 +48,7 @@ $include-iframe-type: true !default; $iframe-padding-top: 40px !default; $iframe-background: #000 !default; $iframe-max-width: 900px !default; -$iframe-ratio: 9/16 !default; +$iframe-ratio: math.div(9, 16) !default; // Image-type options $include-image-type: true !default; diff --git a/app/assets/stylesheets/common/components/user-card.scss b/app/assets/stylesheets/common/components/user-card.scss index 8e7f2c1a49..b0224c7ced 100644 --- a/app/assets/stylesheets/common/components/user-card.scss +++ b/app/assets/stylesheets/common/components/user-card.scss @@ -1,3 +1,5 @@ +@use "sass:math"; + $card_width: 580px; $avatar_width: 120px; $avatar_margin: -50px; // negative margin makes avatars extend above cards @@ -265,7 +267,7 @@ $avatar_margin: -50px; // negative margin makes avatars extend above cards color: var(--primary); .d-icon { margin: auto; - font-size: $avatar_width / 1.5; + font-size: math.div($avatar_width, 1.5); } &.rounded { border-radius: 50%; diff --git a/app/assets/stylesheets/common/foundation/math.scss b/app/assets/stylesheets/common/foundation/math.scss index 4d57257545..02dd4aceea 100644 --- a/app/assets/stylesheets/common/foundation/math.scss +++ b/app/assets/stylesheets/common/foundation/math.scss @@ -4,6 +4,8 @@ // https://github.com/terkel/mathsass // Constants +@use "sass:math"; + $E: 2.718281828459045; $PI: 3.141592653589793; $LN2: 0.6931471805599453; @@ -48,7 +50,7 @@ $SQRT2: 1.4142135623730951; } } @else if $x >= 1 { @while $x >= 1 { - $x: $x / 2; + $x: $x * 0.5; $exp: $exp + 1; } } @@ -59,7 +61,7 @@ $SQRT2: 1.4142135623730951; // @param {Number} $x // @param {Number} $exp @function ldexp($x, $exp) { - $b: if($exp >= 0, 2, 1 / 2); + $b: if($exp >= 0, 2, 1 * 0.5); @if $exp < 0 { $exp: $exp * -1; } @@ -80,11 +82,11 @@ $SQRT2: 1.4142135623730951; // log(10) // 2.30259 @function log($x) { @if $x <= 0 { - @return 0 / 0; + @return math.div(0, 0); } - $k: nth(frexp($x / $SQRT2), 2); - $x: $x / ldexp(1, $k); - $x: ($x - 1) / ($x + 1); + $k: nth(frexp(math.div($x, $SQRT2)), 2); + $x: math.div($x, ldexp(1, $k)); + $x: math.div($x - 1, $x + 1); $x2: $x * $x; $i: 1; $s: $x; @@ -93,7 +95,7 @@ $SQRT2: 1.4142135623730951; $x: $x * $x2; $i: $i + 2; $sp: $s; - $s: $s + $x / $i; + $s: $s + math.div($x, $i); } @return $LN2 * $k + 2 * $s; } @@ -115,7 +117,7 @@ $SQRT2: 1.4142135623730951; $exp: floor($exp * 0.5); $base: $base * $base; } - @return if($s != 0, 1 / $r, $r); + @return if($s != 0, math.div(1, $r), $r); } // Returns E^x, where x is the argument, and E is Euler's constant, the base of the natural logarithms. @@ -126,7 +128,7 @@ $SQRT2: 1.4142135623730951; @function exp($x) { $ret: 0; @for $n from 0 to 24 { - $ret: $ret + ipow($x, $n) / fact($n); + $ret: $ret + math.div(ipow($x, $n), fact($n)); } @return $ret; } @@ -158,7 +160,7 @@ $SQRT2: 1.4142135623730951; } $ret: 1; @for $i from 1 through 24 { - $ret: $ret - (pow($ret, 2) - $x) / (2 * $ret); + $ret: $ret - math.div(pow($ret, 2) - $x, 2 * $ret); } @return $ret; } diff --git a/app/assets/stylesheets/common/foundation/mixins.scss b/app/assets/stylesheets/common/foundation/mixins.scss index 1a8c2f8fef..118bded6ec 100644 --- a/app/assets/stylesheets/common/foundation/mixins.scss +++ b/app/assets/stylesheets/common/foundation/mixins.scss @@ -5,6 +5,8 @@ // Media queries // -------------------------------------------------- +@use "sass:math"; + $breakpoints: ( mobile-small: 320px, mobile-medium: 350px, @@ -162,7 +164,7 @@ $hpad: 0.65em; $encoded: ""; $slice: 2000; $index: 0; - $loops: ceil(str-length($svg) / $slice); + $loops: ceil(math.div(str-length($svg), $slice)); @for $i from 1 through $loops { $chunk: str-slice($svg, $index, $index + $slice - 1); diff --git a/app/assets/stylesheets/common/foundation/variables.scss b/app/assets/stylesheets/common/foundation/variables.scss index 7ac2a24d31..d6ef33e620 100644 --- a/app/assets/stylesheets/common/foundation/variables.scss +++ b/app/assets/stylesheets/common/foundation/variables.scss @@ -6,6 +6,8 @@ // Layout dimensions // -------------------------------------------------- +@use "sass:math"; + $small-width: 800px !default; $medium-width: 995px !default; $large-width: 1110px !default; @@ -165,7 +167,7 @@ $box-shadow: ( // Uses an approximation of sRGB blending, GAMMA=2 instead of GAMMA=2.2 @function srgb-scale($foreground, $background, $percent) { - $ratio: ($percent / 100%); + $ratio: math.div($percent, 100%); $iratio: 1 - $ratio; $f_r2: red($foreground) * red($foreground); $f_g2: green($foreground) * green($foreground); diff --git a/app/assets/stylesheets/common/select-kit/flair-row.scss b/app/assets/stylesheets/common/select-kit/flair-row.scss index 224ce3836d..f47142da2f 100644 --- a/app/assets/stylesheets/common/select-kit/flair-row.scss +++ b/app/assets/stylesheets/common/select-kit/flair-row.scss @@ -1,3 +1,5 @@ +@use "sass:math"; + $flair-size: 18px; .select-kit.flair-chooser { @@ -15,14 +17,14 @@ $flair-size: 18px; width: $flair-size; &.rounded { - background-size: ($flair-size / 1.4) ($flair-size / 1.4); + background-size: math.div($flair-size, 1.4) math.div($flair-size, 1.4); border-radius: 50%; } .d-icon { display: block; - height: ($flair-size / 1.8); - width: ($flair-size / 1.8); + height: math.div($flair-size, 1.8); + width: math.div($flair-size, 1.8); } } diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index dd8db0e8bf..e7f51ce76f 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -460,7 +460,7 @@ class ThemeField < ActiveRecord::Base else self.error = nil unless error.nil? end - rescue SassC::SyntaxError => e + rescue SassC::SyntaxError, SassC::NotRenderedError => e self.error = e.message unless self.destroyed? end self.compiler_version = Theme.compiler_version diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb index a2274a7792..536249ae6d 100644 --- a/lib/stylesheet/compiler.rb +++ b/lib/stylesheet/compiler.rb @@ -60,6 +60,7 @@ module Stylesheet theme_field: options[:theme_field], color_scheme_id: options[:color_scheme_id], load_paths: load_paths, + validate_source_map_path: false, ) result = engine.render diff --git a/lib/stylesheet/manager/builder.rb b/lib/stylesheet/manager/builder.rb index 9e022b8d59..47a846e140 100644 --- a/lib/stylesheet/manager/builder.rb +++ b/lib/stylesheet/manager/builder.rb @@ -47,7 +47,7 @@ class Stylesheet::Manager::Builder color_scheme_id: @color_scheme&.id, load_paths: load_paths, ) - rescue SassC::SyntaxError => e + rescue SassC::SyntaxError, SassC::NotRenderedError => e if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s) # no special errors for theme, handled in theme editor ["", nil] diff --git a/spec/lib/stylesheet/compiler_spec.rb b/spec/lib/stylesheet/compiler_spec.rb index cc00cbeee1..3334879d3e 100644 --- a/spec/lib/stylesheet/compiler_spec.rb +++ b/spec/lib/stylesheet/compiler_spec.rb @@ -164,7 +164,7 @@ RSpec.describe Stylesheet::Compiler do expect(css).to include("--header_background: #f8745c") expect(css).to include("--header_primary: #88af8e") - expect(css).to include("--header_background-rgb: 248,116,92") + expect(css).to include("--header_background-rgb: 248, 116, 92") end context "with a plugin" do diff --git a/spec/lib/stylesheet/importer_spec.rb b/spec/lib/stylesheet/importer_spec.rb index e11ccd4e55..96d422ec3e 100644 --- a/spec/lib/stylesheet/importer_spec.rb +++ b/spec/lib/stylesheet/importer_spec.rb @@ -118,7 +118,7 @@ RSpec.describe Stylesheet::Importer do styles = Stylesheet::Importer.new({ theme_id: theme.id }).import_color_definitions expect(styles).to include("Color definitions from Child Theme") expect(styles).to include("--custom-color: red") - expect(styles).to include("--custom-color-rgb: 0,0,128") + expect(styles).to include("--custom-color-rgb: 0, 0, 128") end it "should include default theme color definitions" do diff --git a/spec/lib/stylesheet/manager_spec.rb b/spec/lib/stylesheet/manager_spec.rb index dc74ab3a09..0442cd69dc 100644 --- a/spec/lib/stylesheet/manager_spec.rb +++ b/spec/lib/stylesheet/manager_spec.rb @@ -680,8 +680,8 @@ RSpec.describe Stylesheet::Manager do manager: manager, ).compile(force: true) - expect(stylesheet).not_to include("--primary: #c00;") - expect(stylesheet).to include("--primary: #222;") # from base scheme + expect(stylesheet).not_to include("--primary: #CC0000;") + expect(stylesheet).to include("--primary: #222222;") # from base scheme end it "uses the correct scheme when a valid scheme id is used" do @@ -726,7 +726,7 @@ RSpec.describe Stylesheet::Manager do stylesheet2 = builder2.compile expect(stylesheet).not_to eq(stylesheet2) - expect(stylesheet2).to include("--primary: #c00;") + expect(stylesheet2).to include("--primary: #CC0000;") end it "includes updated font definitions" do diff --git a/spec/models/theme_field_spec.rb b/spec/models/theme_field_spec.rb index 145d6fd48a..7098184d57 100644 --- a/spec/models/theme_field_spec.rb +++ b/spec/models/theme_field_spec.rb @@ -160,7 +160,7 @@ HTML field.value = "@import 'missingfile';" field.save! field.ensure_baked! - expect(field.error).to include("File to import not found or unreadable: missingfile") + expect(field.error).to include("Error: Can't find stylesheet to import.") field.value = "body {color: blue};" field.save! diff --git a/spec/services/email_style_updater_spec.rb b/spec/services/email_style_updater_spec.rb index 77742457b6..520edc795a 100644 --- a/spec/services/email_style_updater_spec.rb +++ b/spec/services/email_style_updater_spec.rb @@ -47,7 +47,7 @@ RSpec.describe EmailStyleUpdater do false, ) expect(updater.errors).to_not be_empty - expect(updater.errors.first).to include("Invalid CSS after") + expect(updater.errors.first).to include('Error: expected "}".') expect_settings_to_be_unset end end