Skip to content

Switch to DartSass #32

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 7, 2022
Merged

Switch to DartSass #32

merged 4 commits into from
Sep 7, 2022

Conversation

@tomchipchase
Copy link
Contributor

This swaps sassc for dartsass.

  • Make the spec more generic for handling output from both sassc and dartsass
  • Install dartsass-rails as a dependency. We want to use this gem in a rails application. dartsass-rails packages the binary rather than depending on another gem to do that. We want to use the one there, and not have to pull in a different one, even though we'd have fewer dependencies here.
  • Swap out sassc for a thin wrapper around the dartsass binary.
  • Remove the sassc gem.

@tomchipchase
Make invalid sass spec more generic
0f19343 
In preparation for dartsass. Dartsass and sassc report the same error,
but in a slightly different format.

This updates the spec to make sure the important information is there,
but it doesn't care where in the message it is. This should pass whether
we're using dartsass or sassc
@tomchipchase
Install dartsass-rails as a dependency
b6a95bb 
Despite its name, it doesn't actually pull in all of rails. Its just a
wrapper around the dartsass binary with some useful rake tasks. In this
project we only care about the binary, but using this as a dependency
means we only pull in one dartsass binary when this is used in a rails
application.
@tomchipchase
Swap SassC for DartSass
6394d92
@tomchipchase
Remove sassc as a dependency
db7b01e
@tomchipchase tomchipchase requested review from a team, geoffreymm, ianmooney, lluzak and theocodes and removed request for theocodes September 6, 2022 16:59
sgerrand
sgerrand approved these changes Sep 6, 2022
View reviewed changes
Copy link
Contributor

@sgerrand sgerrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@tomchipchase tomchipchase merged commit 6439dac into main Sep 7, 2022
@tomchipchase tomchipchase deleted the tc/dartsass branch September 7, 2022 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ianmooney ianmooney Awaiting requested review from ianmooney

@geoffreymm geoffreymm Awaiting requested review from geoffreymm

@lluzak lluzak Awaiting requested review from lluzak

1 more reviewer

@sgerrand sgerrand sgerrand approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tomchipchase @sgerrand