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

commented Mar 16, 2019

This closes rails/rails#32896 and #420

@guilleiguaran

This comment has been minimized.

Copy link
Member Author

commented Mar 18, 2019

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

@kaspth

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

Fine by me 🙏

@DeeDeeG

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

commented Mar 20, 2019

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

This comment has been minimized.

Copy link
Member Author

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Member Author

commented Mar 20, 2019

@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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Member Author

commented Mar 20, 2019

@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 Mar 24, 2019

@markets markets referenced this pull request May 22, 2019
jfly added a commit to jfly/worldcubeassociation.org that referenced this pull request Jul 30, 2019
Switch from `rails-sass` to `rails-sassc`
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
Switch from `rails-sass` to `rails-sassc`
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
Switch from `rails-sass` to `rails-sassc`
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
Switch from `rails-sass` to `rails-sassc`
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 added a commit to solidusio/solidus that referenced this pull request Aug 22, 2019
Ensure return from CSS function
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 added a commit to solidusio/solidus that referenced this pull request Aug 22, 2019
Ensure return from CSS function
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 added a commit to solidusio/solidus that referenced this pull request Aug 22, 2019
Ensure return from CSS function
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.