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

upgrade using sass-migrator division **/*.scss #705

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BillRaymond
Copy link

SASS is deprecating the ability to use a / (slash) for division. See the documentation here.

Per the documentation, use Automatic Migration with the following code:

npm install -g sass-migrator
sass-migrator division **/*.scss

I ran this code in the _sass folder. One line was added, and one line was modified. DART SASS warnings no longer appear.

@ashmaroli
Copy link
Member

ashmaroli commented Jan 17, 2023

Thank you for proposing this change, @BillRaymond. Unfortunately, this is not an acceptable proposal even for a project currently open to breaking changes.

If I am not mistaken, list.slash is not compatible with Ruby Sass or C Sass. Therefore, accepting this change means forcing all consumers to use Dart Sass, which I don't intend to.

Technically, the deprecation can be ignored in this case because it is serving as a "separator" which is future-compatible.

For your own use, you have a couple of options to workaround this situation:

  • Switch off the deprecation warning without making changes to stylesheet partials by using a configuration option (mentioned in the article above and in the README for jekyll-sass-converter.
  • Lock to an older version of jekyll-sass-converter via Gemfile.
  • Override the base.scss partial at your end only.

@BillRaymond
Copy link
Author

Thank you for that, @ashmaroli. I think the issue here is when I was installing Ruby (with rbenv) and then created my Gemfile, I added gem "jekyll-sass-converter". When I ran my bundle install and bundle update, it used Dart Sass by default. Is it good enough to simply add gem "jekyll-sass-converter" to my Gemfile to resolve this? Of course running a bundle install and bundle update again.

@ashmaroli
Copy link
Member

Is it good enough to simply add gem "jekyll-sass-converter" to my Gemfile to resolve this?

No @BillRaymond.
To resolve this via Gemfile alone, you should point to an older version:

gem "jekyll-sass-converter", "< 3.0"

In the absence of a proper version directive, Bundler will resolve to the latest version.

@BillRaymond
Copy link
Author

Thank you for the update on this, @ashmaroli. This topic may morph into a different request (see below), but I will mention it here for your review. I am creating a base Docker Ubuntu image that I can use for all my projects. I create the image using the following steps:

  1. Ubuntu 20.4
  2. Use rbenv to select the Ruby version (3.1.2 in this case)
  3. Install Jekyll Bundler

I am using rbenv instead of using apt to install ruby-full based on this conversation between you and michaelbach.

If you want to see the whole image that contains other dependencies I like to have, you can see the Dockerfile here.

When I create a new container from that image, I go to the repo where I want to use the Docker file and type:

jekyll new my-blog . --force

That action, of course, creates the new Jekyll site, but it does not add the dependency you recommend, which is gem "jekyll-sass-converter", "< 3.0". Instead, the image uses Dart as the default SASS processor (which is what I believe is happening).

I guess this leaves me to ponder two questions:

  1. Should the jekyll new command add that specific dependency to avoid using DART? That would also remove the DART deprecation warning messages.
  2. Should Jekyll move to DART? I know very little about this, so please excuse any assumptions.
  3. Should the Jekyll documentation be updated to include a line that adds gem "jekyll-sass-converter", "< 3.0" to the Gemfile?
  4. As an aside from the sass converter conversation, should the docs be updated to use rbenv rather than apt to install Jekyll?

FYI, if you need documentation updates, I am happy to help with that.

@ashmaroli
Copy link
Member

When I create a new container from that image, I go to the repo where I want to use the Docker file and type:

jekyll new my-blog . --force 

There is no need of a period after my-blog in the incantation. --force is only necessary if my-blog would be non-empty. Since you're creating a new Docker container, I am assuming --force is redundant.

That action, of course, creates the new Jekyll site, but it does not add the dependency you recommend

That is expected because jekyll new currently doesn't make assumptions regarding the need for an older version of the sass converter. My recommendation from previous comment is to be made by end-user discretion. By default, jekyll new will automatically run bundle install and install the latest version of the sass-converter. The end-user may either subsequently make the recommended edit to their Gemfile and run bundle install again OR invoke jekyll new with the flag skip-bundle, make the edit to the Gemfile and then run bundle install
This can be automated as well. In docker-speak, this may roughly translate to:

RUN jekyll new my-blog --skip-bundle
RUN cd my-blog
RUN echo 'gem "jekyll-sass-converter", "< 3.0"' >> Gemfile
RUN bundle install
RUN cd .. # or cd -  or whatever to return to previous $PWD

Should the jekyll new command add that specific dependency to avoid using DART?

No. jekyll new will only be updated so if the build fails. IMHO, deprecation warnings are actionable and shouldn't be avoided. Yes, they are noise and therefore distracting. The end-user has the option to turn-off the noise by having quiet_deps: true in their sass configuration. They have the option to avoid dart-sass entirely by downgrading to an older version. All of these options are at users' discretion.

Should the Jekyll documentation be updated to include a line that adds gem "jekyll-sass-converter", "< 3.0" to the Gemfile?

I guess you can add this to the "Troubleshooting" page under jekyllrb.com/docs/.

As an aside from the sass converter conversation, should the docs be updated to use rbenv rather than apt to install Jekyll?

Unfortunately, that is not for me to decide. I suggest you open a pull request at the jekyll/jekyll repository and tag @mattr- for their input.

Finally, regarding the dependencies in your Dockerfile, I doubt bison is really a Jekyll pre-requisite.

@BillRaymond
Copy link
Author

Hi @ashmaroli,

There is no need of a period after my-blog in the incantation. --force is only necessary if my-blog would be non-empty. Since you're creating a new Docker container, I am assuming --force is redundant.

This is just a matter of process I think. I always create a GitHub repo that represents a Jekyll site. I enable the readme option and include the Jekyll .gitignore template when doing so. When I clone the repo, I already cd into it, so that is why the jekyll new . --force.

The end-user may either subsequently make the recommended edit to their Gemfile and run bundle install again OR invoke jekyll new with the flag skip-bundle, make the edit to the Gemfile and then run bundle install This can be automated as well. In docker-speak, this may roughly translate to:

Should the jekyll new command add that specific dependency to avoid using DART?

No. jekyll new will only be updated so if the build fails. IMHO, deprecation warnings are actionable and shouldn't be avoided. Yes, they are noise and therefore distracting. The end-user has the option to turn-off the noise by having quiet_deps: true in their sass configuration. They have the option to avoid dart-sass entirely by downgrading to an older version. All of these options are at users' discretion.

Okay, I understand this process and your reasoning. The reason I suggested updating the Jekyll code to include gem "jekyll-sass-converter", "< 3.0"' is because without that, we get DART as the default and according to this documentation, there could be a breaking change. That could impact themes, such as Minima, that use division. My concern is we will run into another webrick situation where the Jekyll Talk forums are inundated with breaking errors and the response is "update your Gemfile". That is certainly reasonable, but I am looking ahead at what might come when the feature is completely depracated.

Should the Jekyll documentation be updated to include a line that adds gem "jekyll-sass-converter", "< 3.0" to the Gemfile?

I guess you can add this to the "Troubleshooting" page under jekyllrb.com/docs/.

Okay, thank you. Once I get a free moment I will propose the change.

As an aside from the sass converter conversation, should the docs be updated to use rbenv rather than apt to install Jekyll?

Unfortunately, that is not for me to decide. I suggest you open a pull request at the jekyll/jekyll repository and tag @mattr- for their input.

Okay, thank you. Once I get a free moment I will propose the change.

Finally, regarding the dependencies in your Dockerfile, I doubt bison is really a Jekyll pre-requisite.

Okay, thank you. I will try that. I think my dependencies started stringing together after reading far too many articles on setting up Ubuntu and Ruby on a Docker image that uses multi-arch :-). I will try removing Bison to see if there are any issues on ARM64 or AMD64.

I appreciate all the thoughtful replies!

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.

None yet

2 participants