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

Make sass-rails an wrapper for sassc-rails to allow a smooth upgrade #424

Merged
merged 2 commits into from
Mar 24, 2019

Conversation

guilleiguaran
Copy link
Member

@guilleiguaran guilleiguaran commented Mar 16, 2019

This closes rails/rails#32896 and #420

@guilleiguaran
Copy link
Member Author

If there aren't any objections before of the weekend I will merge this and release a new beta version of sass-rails.

@kaspth
Copy link
Contributor

kaspth commented Mar 18, 2019

Fine by me 🙏

@DeeDeeG
Copy link

DeeDeeG commented Mar 18, 2019

As just a typical user of Rails, I think this provides a super easy transition to sassc. I'm also glad it will (I hope) help lots of other people do the same. I'm all for it. Thanks for this PR! 👍

@jeffnyman
Copy link

I apologize for my ignorance here but as someone relatively new to Rails projects, I'm curious as to what the course of action should be. I have the following line in my Gemfile (using a project set up on Rails 5.2.2.1):

gem 'sass-rails', '~> 5.0'

I get the post install message from the sass gem regarding the deprecation.

Does this wrapper mean that I don't need to change the above line to this:

gem 'sassc-rails'

Or should I still make that change anyway?

@guilleiguaran
Copy link
Member Author

guilleiguaran commented Mar 20, 2019

@jeffnyman hey!

This will be released as 6.0 so in your case you will need to remove the version restriction on the Gemfile if you want to get the next version of sass-rails.

You can change the dependency manually to sassc-rails if you want to get SassC right now (since this will be released first as Beta), this PR will just make to sass-rails an "alias" for sassc-rails so everyone using sass-rails or sassc-rails will be using SassC in the future.

@DeeDeeG
Copy link

DeeDeeG commented Mar 20, 2019

I browsed the files of this repository, as changed with this pull request, to see what is still left: https://github.com/rails/sass-rails/tree/706526d411d2d78e578c7944e873ece28e65ed42

It doesn't appear to do very much anymore other than:

  • Require the gem 'sassc-rails', '>= 2.1.0' (see sassc-rails.gemspec)
    • This will ensure you have all the functionality of sassc-rails
  • load sassc on your Rails server (see the contents of the lib folder).

Meanwhile, sassc-rails is already designed to be a drop-in replacement for (the non-wrapperized version of) this gem, sass-rails.

I myself am not an expert, so I may have missed something or misunderstood the details.

@guilleiguaran
Copy link
Member Author

@DeeDeeG that's right, we just want to move automatically to folks using this lib to use sassc instead of raising an warning saying that this is deprecated and that they should move manually.

@DeeDeeG
Copy link

DeeDeeG commented Mar 20, 2019

And I think sassc-rails loads sassc on the Rails server as well. Should the lib folder of this repo be removed in this PR? Or to satisfy my curiosity: Can I ask what the files in the lib folder do that is better than deleting it? (Would something likely break?)

Thanks for being available to discuss and answer questions @guilleiguaran

@guilleiguaran
Copy link
Member Author

@DeeDeeG

I'm keeping the files for the folks that are requiring them directly, e.g:

require "sass/rails/importer"

Instead of raising an error in that case we are requiring the corresponding file in sassc-rails, in this case sassc/rails/importer

@guilleiguaran guilleiguaran merged commit 409d871 into master Mar 24, 2019
@guilleiguaran guilleiguaran deleted the sassc-rails branch March 24, 2019 22:28
jfly added a commit to jfly/worldcubeassociation.org that referenced this pull request Jul 30, 2019
This is a step towards thewca#3649 =)

As far as I can tell, this is a drop in replacement. In fact, it looks
like `rails-sass` is already a meaningless shell around `rails-sassc`:
rails/sass-rails#424, so I think this is
basically a no-op. Regardless, it feels cleaner to me to use
`rails-sassc` directly.
jfly added a commit to jfly/worldcubeassociation.org that referenced this pull request Jul 30, 2019
This is a step towards thewca#3649 =)

As far as I can tell, this is a drop in replacement. In fact, it looks
like `rails-sass` is already a meaningless shell around `rails-sassc`:
rails/sass-rails#424, so I think this is
basically a no-op. Regardless, it feels cleaner to me to use
`rails-sassc` directly.
jfly added a commit to jfly/worldcubeassociation.org that referenced this pull request Jul 31, 2019
This is a step towards thewca#3649 =)

As far as I can tell, this is a drop in replacement. In fact, it looks
like `rails-sass` is already a meaningless shell around `rails-sassc`:
rails/sass-rails#424, so I think this is
basically a no-op. Regardless, it feels cleaner to me to use
`rails-sassc` directly.
jfly added a commit to thewca/worldcubeassociation.org that referenced this pull request Jul 31, 2019
This is a step towards #3649 =)

As far as I can tell, this is a drop in replacement. In fact, it looks
like `rails-sass` is already a meaningless shell around `rails-sassc`:
rails/sass-rails#424, so I think this is
basically a no-op. Regardless, it feels cleaner to me to use
`rails-sassc` directly.
kennyadsl pushed a commit to solidusio/solidus that referenced this pull request Aug 22, 2019
According to the `sassc` compiler, all CSS functions should ensure that
they do not end without a `@return` statement. The `very-light` function
has the possibility that it could exit without calling such a statement,
so we add a catchall `@return` at the end to return the unmodified color
value.

We need this commit since sass-rails is a wrapper for sassc-rails
starting from version 6.0. This means that new apps (or apps that upgrade
that gem) will use sassc by default.

rails/sass-rails#424
kennyadsl pushed a commit to solidusio/solidus that referenced this pull request Aug 22, 2019
According to the `sassc` compiler, all CSS functions should ensure that
they do not end without a `@return` statement. The `very-light` function
has the possibility that it could exit without calling such a statement,
so we add a catchall `@return` at the end to return the unmodified color
value.

We need this commit since sass-rails is a wrapper for sassc-rails
starting from version 6.0. This means that new apps (or apps that upgrade
that gem) will use sassc by default.

rails/sass-rails#424
kennyadsl pushed a commit to solidusio/solidus that referenced this pull request Aug 22, 2019
According to the `sassc` compiler, all CSS functions should ensure that
they do not end without a `@return` statement. The `very-light` function
has the possibility that it could exit without calling such a statement,
so we add a catchall `@return` at the end to return the unmodified color
value.

We need this commit since sass-rails is a wrapper for sassc-rails
starting from version 6.0. This means that new apps (or apps that upgrade
that gem) will use sassc by default.

rails/sass-rails#424
rokumatsumoto added a commit to rokumatsumoto/boyutluseyler that referenced this pull request Dec 8, 2019
- Switch to sassc-rails gem until we upgrade Rails to 6.0
- Update babel.config.js file
- Bump lodash to 4.17.15

rails/sass-rails#420
rails/sass-rails#424
rails/webpacker@6dfcad6
https://github.com/rails/webpacker/pull/2247/files
@unicornrainbow
Copy link

Sorry, I don't agree. Sass-rails should handle integration of sass into rails.

If that means the sass gem, then it need not to be concerned with the sassc integration or gem, and if some EOL dictum is to be accepted being handed down from on high, then it should be EOL'd too.

If sass means SASS! Then it should try to handle the multiple Sass engines that exists and are desired to be used. Even perhaps ones that may be "EOL", tra la la. But there are at least 3, and LibSass says it is deprecated now, and is directing readme reader's to use Dart Sass. It's a wild goose chase, and honestly can be very easily solved by allowing the user to pick their engine, having the Rails integration code centralized in a single place, perhaps most appropriately this repository (strange it would be to load the ruby sass gem or Dart sass from the sassc-rails gem, no?).

Side note: Talk of EOL and deprecation is a bit silly because nobody is really asking for much, it works, and if people want to make it better, they should feel free to do so. It's opensource.

So, things as they are, this is essentially a dead project. But rather it could be a very beneficial project if it would outright handle the integration across multiple Sass engines into Rails.

To do this, it shouldn't have a dependency on any of the engines. The user can add the engine they want by adding its Gem, which Sass-Rails could sniff out and then configure automatically.

If multiple engines are loaded, then there could be a preference order imposed, and the user should also have the means to explicitly specify the Gem/engine to use.

Leaving it there, here's a 🥪.

Good luck and gravitas!

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.

Ruby Sass End-of-Life: 26 March 2019
5 participants