Skip to content
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

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Conversation

floehopper
Copy link
Contributor

@floehopper floehopper commented Jan 22, 2024

Since LibSass is now deprecated, this is a change that needs making anyway. However, my actual motivation is that I'm hoping it will fix it fixes a SassC::SyntaxError that I'm seeing after adding govuk_publishing_components when running RAILS_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 run RAILS_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.

floehopper added a commit to alphagov/govuk-docker that referenced this pull request Jan 22, 2024
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
@floehopper floehopper marked this pull request as draft January 22, 2024 14:01
@floehopper floehopper force-pushed the move-from-libsass-to-dart-sass branch 3 times, most recently from 0846912 to 39500a2 Compare January 22, 2024 14:11
@floehopper floehopper marked this pull request as ready for review January 22, 2024 14:14
procfile.dev Outdated Show resolved Hide resolved
procfile.dev Outdated Show resolved Hide resolved
bin/dev Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
floehopper added a commit to alphagov/govuk-docker that referenced this pull request Jan 22, 2024
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
@floehopper floehopper force-pushed the move-from-libsass-to-dart-sass branch from 39500a2 to 610e25a Compare January 22, 2024 16:37
@floehopper
Copy link
Contributor Author

However, my actual motivation is that I'm hoping it will fix a SassC::SyntaxError that I'm seeing after adding govuk_publishing_components when running RAILS_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 have now rebased the branch in this draft PR against this branch and confirmed that it does indeed fix the SassC::SyntaxError. 🎉 I've updated this PR description accordingly.

Copy link
Contributor

@AgaDufrat AgaDufrat left a 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.

@floehopper floehopper merged commit c7c85b6 into main Jan 24, 2024
11 checks passed
@floehopper floehopper deleted the move-from-libsass-to-dart-sass branch January 24, 2024 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants