-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move from libsass -> dart-sass #1253
Conversation
When I moved [1] the support 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 on port 3070 instead of port 3000. This updates the `docker-compose.yml` file accordingly. [1]: alphagov/support#1253 [2]: https://docs.publishing.service.gov.uk/manual/migrate-to-dart-sass-from-libsass.html
0846912
to
39500a2
Compare
When I moved [1] the support 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/support#1253 [2]: https://docs.publishing.service.gov.uk/manual/migrate-to-dart-sass-from-libsass.html
I followed these instructions [1]. I'm hoping that this will fix a `SassC::SyntaxError` that I'm seeing after adding `govuk_publishing_components` when running `RAILS_ENV=production rails assets:precompile`. Note that running `bundle install` has removed the `ruby` platform. We think this is because `sass-embedded`, a dependency of `dartsass-rails`, doesn't have a non-platform-specific variant of the gem and so bundler can no longer support the `ruby` platform. [1]: https://docs.publishing.service.gov.uk/manual/migrate-to-dart-sass-from-libsass.html
39500a2
to
610e25a
Compare
I have now rebased the branch in this draft PR against this branch and confirmed that it does indeed fix the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM. Tested locally with the branch of docker as well.
Since LibSass is now deprecated, this is a change that needs making anyway. However, my actual motivation is that
I'm hoping it will fixit fixes aSassC::SyntaxError
that I'm seeing after addinggovuk_publishing_components
when runningRAILS_ENV=production rails assets:precompile
in order to start moving the app to use the GOV.UK Design System. See this build failure which occurred against the code in this draft PR.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.
I've tested this locally using the modified version of
govuk-docker
and verified that I can make changes to CSS and see them reflected in the rendered pages. I've also runRAILS_ENV=production rails assets:precompile
locally and I don't see any errors or warnings. However, note that the instructions told me to suppress warnings in dependencies and before I did that, I did see some deprecation warnings before I did that which doesn't seem ideal.