Skip to content

removed sassc gem; everything should now be working from dart-sass#200

Closed
LeslieJAnderson wants to merge 2 commits intomainfrom
rm-sassc-and-move-to-dart-sass
Closed

removed sassc gem; everything should now be working from dart-sass#200
LeslieJAnderson wants to merge 2 commits intomainfrom
rm-sassc-and-move-to-dart-sass

Conversation

@LeslieJAnderson
Copy link
Copy Markdown

@LeslieJAnderson LeslieJAnderson commented Apr 23, 2025

Ticket

Resolves navapbc/template-application-rails#87

Changes

the sassc gem was removed.

Context for reviewers

We needed to remove the deprecated sassc gem, in favor of dart-sass. It appeared dart-sass was already in play and nothing else needed to change.

Testing

I logged into the app and clicked around with an eye out for css issues; none were apparent.

Screenshot 2025-04-23 at 2 59 46 PM

Preview environment for app

♻️ Environment destroyed ♻️

Preview environment for app-rails

♻️ Environment destroyed ♻️

@LeslieJAnderson LeslieJAnderson force-pushed the rm-sassc-and-move-to-dart-sass branch from ce916e7 to 06f1c3f Compare April 23, 2025 18:57
@LeslieJAnderson LeslieJAnderson marked this pull request as ready for review April 23, 2025 19:52
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

wow that's all that needed to be done? i'm... surprised this works. where is dart-sass included? is it bundled as part of rails?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Seems that way! 🤞

So, I traced the stylesheet_link_tag to application.scss. That file uses the Dart Sass module system (@use, @forward) which sassc cannot compile -- it'd throw a syntax error.

As for "where is dart-sass?" -- the project uses the official sass npm package, which is Dart Sass, so the build:css script in package.json should explicitly call the Dart Sass CLI to compile application.scss.

Since everything is compiling and looks correct, I'm assuming the groundwork was already laid, and the sassc gem was still just lingering and causing problems.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ooh, so the npm package sass in package.json is dart sass. didn't realize that. amazing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@LeslieJAnderson actually one more thing, is "cssbundling-rails" still needed?

Copy link
Copy Markdown
Author

@LeslieJAnderson LeslieJAnderson Apr 24, 2025

Choose a reason for hiding this comment

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

Yeah, cssbundling-rails takes care of all the glue work between Rails and the Node-based CSS toolchain; without it we'd need to do a bunch of work to line things up

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh cool thanks. btw process nit but can you change the PR title to adhere to git commit message naming conventions (https://cbea.ms/git-commit/) i.e. use present tense, imperative voice

Copy link
Copy Markdown
Collaborator

@lorenyu lorenyu left a comment

Choose a reason for hiding this comment

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

LGTM to move the changes over to template-application-rails

@lorenyu
Copy link
Copy Markdown
Collaborator

lorenyu commented Apr 24, 2025

@LeslieJAnderson I went ahead and made this change upstream since I have other changes that depend on this navapbc/template-application-rails#94

@lorenyu lorenyu closed this Apr 24, 2025
@lorenyu lorenyu deleted the rm-sassc-and-move-to-dart-sass branch April 24, 2025 19:43
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.

Update sassc library to use dart-sass

2 participants