Skip to content

Replace ruby-sass with dart-sass (node)

Muhammed Ali requested to merge gitlab-community/gitlab:replace-sass into master

What does this MR do and why?

This MR removes the deprecated Ruby Sass and replaces it with Dart Sass using the Javascript library.

It introduces cssbundling-rails to hook into Sprockets Asset Pipeline. Feedback is welcome, and decisions around architecture and references that would require updating would also be useful

Yes thing is potentially a big change, but with it I hope we can move away from Sprockets3, maybe even skip Sprockets4, and move straight to Propshaft and jsbundling-rails.

Status

See - #438278 (closed)

This MR adds cssbundling-rails alongside sassc-rails. By default sassc-rails continues to be used, but cssbundling-rails can be used by enabling USE_NEW_CSS_PIPELINE env var.

Proposals

  • Introduce a {ee?,jh?}app/assets/stylesheets/entrypoints directory, being the single point where all entrypoint CSS files are defined. Then all the files would have to be moved around, but I presume very little will have to be updated
    • the command would just be sass app/assets/stylesheets/entrypoints/:app/assets/builds while everything inside entrypoints/ has the same structure as it does now
  • Possibly introduce PostCSS as a suitable Node based CSS Processor. Adds some tendency of processing Sass files

Hurdles

  1. sass Javascript API is very poor. Meaning the script for build:css can be used to "compile" production assets. But for things like --watch or the Command Line interface, it's not replicable via the JS API. So we may need to:
  • Write super funky JS to construct a command line command to run, both for production and development
  • Keep build:css for production asset building, and use the JS API to conditionally import EE/JH assets. And use maybe a watch:css script which invokes sass via CLI
  1. Think about future of @import, as Sass is planning on removing it

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Application still looks and should look exactly the same

Screenshots are required for UI changes, and strongly recommended for all other merge requests. image

Before After

Picture from the Review App

image

How to set up and validate locally

Clear your local assets: rm -rf public/assets tmp/cache/assets/. After that a gdk restart rails-web should be sufficient.

Old way of compiling css

This MR should not mess with the status quo, so starting your gdk normally, on this branch should have working styles If you e.g. add body { color: green !important; } to app/assets/stylesheets/application.scss, some text color should change to green after a page reload

New way of compiling css

Do the following in one terminal window:

gdk stop
rm -rf public/assets tmp/cache/assets/
export USE_NEW_CSS_PIPELINE=1
gdk start

At the same time, in another terminal window, do:

yarn run build:css --watch

If you now do change e.g. application.scss, the second terminal window should read: application.scss changed, recompiling and after a page reload the change you did should be effective.

Comparing assets

This MR also includes a script which runs production compilation of the assets in both modes and places two diffable folders in tmp/css_compare. Simply run: ./scripts/frontend/compare_css_compilers.sh. This shows that the production mode is working for both, but we need to figure out the actual diff between the two before we can make the switch.

/cc @leetickett-gitlab

Edited by Lukas Eipert

Merge request reports