sassc-ruby
sassc-ruby copied to clipboard
sass
Replace Libsass dependency with sass-embedded
Combine Natsuki's sassc-embedded-shim-ruby gem with sassc-ruby
TODO:
- [ ] Check if
.sass(not.scss) files work on Rails.
@johnnyshields can you update the PR to allow sassc-embedded 1.55 and above? That version includes native gems, which will make deployment quite easy.
Great work!!! We are super happy to see this as we've been discussing what to do regarding SassC in @discourse for a long time.
This appears to be a drop-in replacement in most cases, but during my tests this appears to be more strict about source maps URLs and files, making quite a few specs fail.
From a cursory look, this appears to be coming from new Ruby code, and not from dart-sass, so can this behavior be behind a knob of some sorts?
@xfalcox given the complexity/magnitude of the change it will be much easier/cleaner to abandon SassC altogether and release this as a new major version. Maintenance releases (if any) for SassC can happen on the old major version branch.
I have been running this branch in production without issues at TableCheck for several months. Let me know what steps I can take to get this PR merged.
@xfalcox given the complexity/magnitude of the change it will be much easier/cleaner to abandon SassC altogether and release this as a new major version. Maintenance releases (if any) for SassC can happen on the old major version branch.
Yeah sure, 100% agree.
What I meant is that I tried doing the upgrade to use this branch, and it works just fine, except for the call at
https://github.com/tablecheck/sassc-ruby/blob/embedded-sass-merge/lib/sassc/url.rb#L35
started by
https://github.com/tablecheck/sassc-ruby/blob/embedded-sass-merge/lib/sassc/engine.rb#L133
which tried to find a valid file path in the source map, which I don't think it's mandatory for the upgrade.
Can we please, if possible, put this path file validation behind a setting so we can disable and make the move to this new version easier?
@xfalcox that sounds fine, can you raise a PR to my branch and I'll merge it?
@ashkulz
can you update the PR to allow sassc-embedded 1.55 and above?
The gemspec currently includes spec.add_runtime_dependency 'sass-embedded', '~> 1.54'. I'd like to leave this as-is as this is what I've tested. It will allow newer versions in the 1.x series, so 1.55 is fine.
@johnnyshields thanks, we're using it successfully in Production. I forgot to update my comment :see_no_evil:
I've also been using it in production for a few months on a large app with lots of scss files with no issue.
@xfalcox that sounds fine, can you raise a PR to my branch and I'll merge it?
Opened https://github.com/tablecheck/sassc-ruby/pull/1. With it I can get all specs to pass in @discourse with minimal changes and then work towards being able to re-enable the source map file validation over time.
@xfalcox looks good, just merged it. I appreciate if anyone can help get this PR merged and officially released on this gem.
@xfalcox looks good, just merged it. I appreciate if anyone can help get this PR merged and officially released on this gem.
Sent another one with a oneliner fix from my previous PR.
This is looking great at @discourse. We are just waiting on this PR to land and a new gem to be released with it.
I suspect that would be @bolandrm, at least that is the only person who currently can make a release, as the sole gem owner at RubyGems.org: https://rubygems.org/gems/sassc
SassC means Sass in C (libsass). In my opinion, you should keep this gem as is and release the dart implementation as a new gem based on this PR and name it appropriately. it's cleaner this way and you can maintain the new gem on your own, but the downside is that dependent gems like https://github.com/rails/sprockets have to be updated as well. It shouldn't be too much work. What do you think?
No, I think it's more gems that would have to be updated to support this. Not sure if something in sprockets has to change but it would in sassc-rails and also sass-rails. More functionality from those gems would have to be duplicated if a whole new gem were to be created to replace that tree. I do think it makes sense to make this change here as a new major release in this gem as a stop gap.
that's what happened with the original https://github.com/sass/ruby-sass gem
sprockets still supports both separated gems on the Rails side, sass was replaced by sassc, see https://github.com/rails/sass-rails/pull/424
I agree it's easier to keep it in this repo instead of duplicating the API to a new fork and forcing users to change the gem, but I don't believe the original and only maintainer will ever merge it, since he's been inactive for more than 2 years...
I'm ready to start using this in production in @discourse. @bolandrm is there any change a major release of the gem can be cut using this? Should we give up on that and look into forking?
@xfalcox looks like its working. You can replace sassc-rails with dartsass-sprockets in your Gemfile.
@xfalcox by the way, it looks like you broke a few tests related to sourcemaps. Please kindly take a look here: https://github.com/tablecheck/dartsass-ruby (sorry CI isn't running yet.)
@johnnyshields thank you! Yesterday, I gave this a try locally and in our GHA CI. So far its worked great in development. Removing sassc has solved our primary issue with Docker builds as installing the gem alone would take ~4 minutes.
Is it possible using either dartsass-ruby or dartsass-sprockets to specify a dart-sass quietDeps flag to silence deprecation warnings? We have many due to foundation sites and emails.
I've noticed some broken links in the repo and on the gemspec from Rubygems so I'll create PRs for that.
@johnnyshields can you please remove the fork relationship on both gems? You should be able to do this through GitHub chatbot https://stackoverflow.com/a/16052845 now. The reason is that I can't create a fork of dartsass-sprockets since it forks sassc-rails instead which already exists on my account.
@javierjulio thanks for the chatbot link, I've raised the ticket for both gems just now.