Use dartsass-rails vs sassc-rails #2709
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
floehopper
added a commit
to alphagov/govuk-docker
that referenced
this pull request
Feb 13, 2024
When I moved [1] the signon app from libsass to dart-sass following these instructions [2], I created a new `bin/dev` file which runs two processes in `Procfile.dev`, one of which runs the rails server and another which runs `dart-sass` in watch mode. This updates the `docker-compose.yml` file accordingly. [1]: alphagov/signon#2709 [2]: https://docs.publishing.service.gov.uk/manual/migrate-to-dart-sass-from-libsass.html
chrisroos
reviewed
Feb 13, 2024
chrisroos
reviewed
Feb 13, 2024
chrisroos
approved these changes
Feb 13, 2024
There was a problem hiding this comment.
Apart from the inclusion of application.css in the builds directory this all looks good to me, @floehopper 👍
This code re-defines the assets-related tasks in order to avoid a load of INFO-level warnings generated by the assets:precompile task [1]. It wasn't immediately obvious to me that was what was happening (even though I made the change!). I think moving the code into a separate `lib/tasks/assets.rake` file is more idiomatic and makes it slightly more obvious what's going on. [1]: #2179
I followed these instructions [1]. [1]: https://docs.publishing.service.gov.uk/manual/migrate-to-dart-sass-from-libsass.html
The `dartsass-rails` gem does this enhancement automatically. However, because we re-define the `assets:precompile` task, we lose the enhancement and have to re-add it here. c.f. alphagov/whitehall@13bd112
This fixes the following deprecation warning:
Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.
Recommendation: math.div($sort-link-arrow-size, 2) or calc($sort-link-arrow-size / 2)
More info and automated migrator: https://sass-lang.com/d/slash-div
floehopper
force-pushed
the
use-dartsass-rails-vs-sassc-rails
branch
from
216c6ea
to
dc16605
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LibSass is now deprecated, so I followed these instructions in the GOV.UK Developer Docs to make the change. Once it's merged, there's also a corresponding change to govuk-docker that will need to me made - see this PR.
This should address the following Bundler post-install message is was seeing:
I've tested this locally using the modified version of
govuk-dockerand verified that I can make changes to CSS and see them reflected in the rendered pages. I've also runRAILS_ENV=production rails assets:precompilelocally and I don't see any errors or warnings.c.f. alphagov/support#1253