Remove configuration of theme sass files from Core (#7290)

Merge pull request 7290

Gemfile

@@ -7,6 +7,10 @@ gemspec :name => "jekyll"
 # refinements introduced in i18n-1.3.0
 gem "i18n", "~> 1.2.0" if RUBY_ENGINE == "jruby"
 
+# Point to the sass-converter's repository to ensure Windows builds render as expected.
+# Remove once "jekyll-sass-converter-2.0.0" ships.
+gem "jekyll-sass-converter", :github => "jekyll/jekyll-sass-converter"
+
 gem "rake", "~> 12.0"
 
 group :development do

features/rendering.feature

@@ -154,7 +154,7 @@ Feature: Rendering
     When I run jekyll build
     Then I should get a zero exit status
     And the _site directory should exist
-    And I should see ".foo-bar {\n  color: red; }" in "_site/index.css"
+    And I should see ".foo-bar { color: red; }\n\n\/\*# sourceMappingURL=index.css.map \*\/" in "_site/index.css"
 
   Scenario: Not render liquid in CoffeeScript without explicitly including jekyll-coffeescript
     Given I have an "index.coffee" page with animal "cicada" that contains "hey='for {{page.animal}}'"

features/theme.feature

@@ -16,12 +16,19 @@ Feature: Writing themes
 
   Scenario: A theme with SCSS
     Given I have a configuration file with "theme" set to "test-theme"
-    And I have a css directory
-    And I have a "css/main.scss" page that contains "@import 'test-theme-black';"
     When I run jekyll build
     Then I should get a zero exit status
     And the _site directory should exist
-    And I should see ".sample {\n  color: black; }" in "_site/css/main.css"
+    And I should see ".sample { color: red; }\n\n\/\*# sourceMappingURL=style.css.map \*\/" in "_site/assets/style.css"
+
+  Scenario: Overriding a theme with SCSS
+    Given I have a configuration file with "theme" set to "test-theme"
+    And I have an assets directory
+    And I have an "assets/style.scss" page that contains "@import 'test-theme-black';"
+    When I run jekyll build
+    Then I should get a zero exit status
+    And the _site directory should exist
+    And I should see ".sample { color: black; }\n\n\/\*# sourceMappingURL=style.css.map \*\/" in "_site/assets/style.css"
 
   Scenario: A theme with an include
     Given I have a configuration file with "theme" set to "test-theme"

jekyll.gemspec

@@ -37,7 +37,7 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency("colorator",             "~> 1.0")
   s.add_runtime_dependency("em-websocket",          "~> 0.5")
   s.add_runtime_dependency("i18n",                  ">= 0.9.5", "< 2")
-  s.add_runtime_dependency("jekyll-sass-converter", "~> 1.0")
+  s.add_runtime_dependency("jekyll-sass-converter", "= 2.0.0.pre.beta")
   s.add_runtime_dependency("jekyll-watch",          "~> 2.0")
   s.add_runtime_dependency("kramdown",              "~> 2.1")
   s.add_runtime_dependency("kramdown-parser-gfm",   "~> 1.0")

lib/jekyll/theme.rb

@@ -10,7 +10,6 @@ module Jekyll
       @name = name.downcase.strip
       Jekyll.logger.debug "Theme:", name
       Jekyll.logger.debug "Theme source:", root
-      configure_sass
     end
 
     def root
@@ -38,13 +37,6 @@ module Jekyll
       @assets_path ||= path_for "assets"
     end
 
-    def configure_sass
-      return unless sass_path
-
-      External.require_with_graceful_fail("sass") unless defined?(Sass)
-      Sass.load_paths << sass_path
-    end
-
     def runtime_dependencies
       gemspec.runtime_dependencies
     end

test/fixtures/test-theme/assets/style.scss

@@ -1,3 +1,3 @@
 ---
 ---
-@import "test-theme-{{ site.theme-color | default: "red" }}";
+@import "test-theme-{{ site.theme-color | default: 'red' }}";

test/test_filters.rb

@@ -138,14 +138,18 @@ class TestFilters < JekyllUnitTest
 
     should "sassify with simple string" do
       assert_equal(
-        "p {\n  color: #123456; }\n",
-        @filter.sassify("$blue:#123456\np\n  color: $blue")
+        "p { color: #123456; }\n",
+        @filter.sassify(<<~SASS)
+          $blue: #123456
+          p
+            color: $blue
+        SASS
       )
     end
 
     should "scssify with simple string" do
       assert_equal(
-        "p {\n  color: #123456; }\n",
+        "p { color: #123456; }\n",
         @filter.scssify("$blue:#123456; p{color: $blue}")
       )
     end
@@ -811,7 +815,7 @@ class TestFilters < JekyllUnitTest
               "The list of grouped items for '' is not an Array."
             )
             # adjust array.size to ignore symlinked page in Windows
-            qty = Utils::Platforms.really_windows? ? 15 : 16
+            qty = Utils::Platforms.really_windows? ? 16 : 18
             assert_equal qty, g["items"].size
           end
         end
@@ -1066,7 +1070,7 @@ class TestFilters < JekyllUnitTest
               "The list of grouped items for '' is not an Array."
             )
             # adjust array.size to ignore symlinked page in Windows
-            qty = Utils::Platforms.really_windows? ? 15 : 16
+            qty = Utils::Platforms.really_windows? ? 16 : 18
             assert_equal qty, g["items"].size
           end
         end

test/test_sass.rb

@@ -14,7 +14,12 @@ class TestSass < JekyllUnitTest
     end
 
     should "import SCSS partial" do
-      assert_equal ".half {\n  width: 50%; }\n", File.read(@test_css_file)
+      result = <<~CSS
+        .half { width: 50%; }
+
+        /*# sourceMappingURL=main.css.map */
+      CSS
+      assert_equal result.rstrip, File.read(@test_css_file)
     end
 
     should "register the SCSS converter" do

test/test_site.rb

@@ -240,6 +240,7 @@ class TestSite < JekyllUnitTest
         index.html
         index.html
         info.md
+        main.css.map
         main.scss
         properties.html
         sitemap.xml
@@ -248,9 +249,9 @@ class TestSite < JekyllUnitTest
       )
       unless Utils::Platforms.really_windows?
         # files in symlinked directories may appear twice
-        sorted_pages.push("main.scss", "symlinked-file").sort!
+        sorted_pages.push("main.css.map", "main.scss", "symlinked-file").sort!
       end
-      assert_equal sorted_pages, @site.pages.map(&:name)
+      assert_equal sorted_pages, @site.pages.map(&:name).sort!
     end
 
     should "read posts" do

test/test_theme.rb

@@ -26,12 +26,6 @@ class TestTheme < JekyllUnitTest
         Theme.new("foo").version
       end
     end
-
-    should "add itself to sass's load path" do
-      @theme.configure_sass
-      message = "Sass load paths should include the theme sass dir"
-      assert Sass.load_paths.include?(@theme.sass_path), message
-    end
   end
 
   context "path generation" do

test/test_theme_assets_reader.rb

@@ -39,7 +39,7 @@ class TestThemeAssetsReader < JekyllUnitTest
       file = @site.pages.find { |f| f.relative_path == "assets/style.scss" }
       refute_nil file
       assert_equal @site.in_dest_dir("assets/style.css"), file.destination(@site.dest)
-      assert_includes file.output, ".sample {\n  color: black; }"
+      assert_includes file.output, ".sample { color: black; }"
     end
 
     should "not overwrite site content with the same relative path" do