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

Remove dependency on sassc / scss #2148

Closed
wants to merge 8 commits into from
Closed

Conversation

muriloime
Copy link

Hello there,

I realized the sassc related gems are increasing our slug size in heroku by 10-20% so I decided to remove the (now deprecated) sassc dependencies. In the case this work is desirable by the maintainers, I am creating this PR

cheers
Murilo

@nickcharlton
Copy link
Member

Thanks for jumping in and doing this. I've been keen to do this for the last few weeks, so I'm glad you've opened a PR about it.

@dcyoung-dev has a work in progress branch hanging around from when we initially looked at this, but it seems like this is a little further along, but I mention it in case you wanted to have a quick look.

Would you be able to do something about the Hound/style comments?

@iainbeeston
Copy link

Any chance of hurrying this along? I have another reason for wanting this PR which is that I'm using Rails 7 with tailwind 3, and adding sassc into the mix causes tailwind to blow up with a [Error: Function rgb is missing argument $green](https://github.com/tailwindlabs/tailwindcss/discussions/6738) error (apparently due to sassc being old/out of date)

@muriloime
Copy link
Author

@iainbeeston this PR broke in production. I added a WIP

@muriloime muriloime changed the title Remove dependency on sassc / scss [WIP] Remove dependency on sassc / scss Mar 30, 2022
@brunoprietog
Copy link

I'm also unable to use Administrate because I'm using Tailwind. It would be great to see this PR merged. Thanks!

@@ -0,0 +1 @@
//= link_tree ../stylesheets/administrate .css
Copy link

Choose a reason for hiding this comment

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

Expected space or tab after '//' in comment spaced-comment

button:disabled:hover,
input[type="button"]:disabled:hover,
input[type="reset"]:disabled:hover,
input[type="submit"]:disabled:hover,
Copy link

Choose a reason for hiding this comment

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

Unexpected qualifying type selector selector-no-qualifying-type

}
button:disabled:hover,
input[type="button"]:disabled:hover,
input[type="reset"]:disabled:hover,
Copy link

Choose a reason for hiding this comment

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

Unexpected qualifying type selector selector-no-qualifying-type

opacity: 0.5;
}
button:disabled:hover,
input[type="button"]:disabled:hover,
Copy link

Choose a reason for hiding this comment

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

Unexpected qualifying type selector selector-no-qualifying-type

cursor: not-allowed;
opacity: 0.5;
}
button:disabled:hover,
Copy link

Choose a reason for hiding this comment

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

Expected empty line before rule rule-empty-line-before

button:disabled,
input[type="button"]:disabled,
input[type="reset"]:disabled,
input[type="submit"]:disabled,
Copy link

Choose a reason for hiding this comment

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

Unexpected qualifying type selector selector-no-qualifying-type

@muriloime
Copy link
Author

muriloime commented Jul 14, 2022

I had a lot of sassc errors in my CI/CD server when running tests so I came back to this issue.

I finally managed to remove sass. Unfortunately I had to change selectize-rails too, as it used .scss.

I acknowledge it is maybe not the best way to use the asset pipeline (multiple files), but it is working now in prod \o/, and it does not rely on scss anymore. I would be more than happy to discuss on how to improve the solution

In order to make it work is necessary to add this line to app's manifest file

//= link administrate_manifest.js

Cheers
Murilo

@muriloime muriloime changed the title [WIP] Remove dependency on sassc / scss Remove dependency on sassc / scss Jul 14, 2022
@czepesch
Copy link

@muriloime hey!
I tried to deploy my app with Dokku using your fork, but have this error:
`-----> Preparing app for Rails asset pipeline
Running: rake assets:precompile

   Done in 1389ms.
   rake aborted!
   LoadError: cannot load such file -- sassc`

I added //= link administrate_manifest.js to manifest,js

I also can't do bundle exec rake assets:precompile locally:
rake aborted! LoadError: cannot load such file -- sassc
Any ideas?
Thanks!

@muriloime
Copy link
Author

@muriloime hey! I tried to deploy my app with Dokku using your fork, but have this error: `-----> Preparing app for Rails asset pipeline Running: rake assets:precompile

   Done in 1389ms.
   rake aborted!
   LoadError: cannot load such file -- sassc`

I added //= link administrate_manifest.js to manifest,js

I also can't do bundle exec rake assets:precompile locally: rake aborted! LoadError: cannot load such file -- sassc Any ideas? Thanks!

Hey actually I had to modify selectize gem too ( and pointed the gem to my github fork). Very simple change, just changed the extension of the relevant files to me muriloime/selectize-rails@130c599.

@czepesch
Copy link

@muriloime I don't have this gem in my project and don't think I should use at all. If so, there is no way to make administrate work? or it is possible to make similar changes somewhere else?

@muriloime
Copy link
Author

muriloime commented Oct 11, 2022 via email

@sodabrew
Copy link

Kudos for this work, would be glad to see it land and reduce deploy sizes substantially

@MuhammetDilmac
Copy link

I think this pr has a urgent priority because of tailwind conflict. Have a expected release date?

@muriloime
Copy link
Author

@MuhammetDilmac @sodabrew I don't know if this PR will ever see the light of day, as it required changes on other gems as well. I use this in production but I had to point to github versions of other gems. If some admin has an idea and are willing to release this change I am more than prone to discuss.

@sodabrew
Copy link

@muriloime was the only other change in selectize-rails? The version of Selectize used by selectize-rails is wildly out of date and looking at https://github.com/manuelvanrijn/selectize-rails, seems unlikely to be updated anytime. I would suggest removing it by importing selectize.js into this PR directly from https://github.com/selectize/selectize.js#installation

Comparing notes across projects, there is a Selectize plugin for ActiveAdmin: https://github.com/blocknotes/activeadmin_selectize (rather than using selectize-rails -- yet also uses sassc at runtime)

@nickcharlton nickcharlton mentioned this pull request Jan 20, 2023
3 tasks
@pablobm
Copy link
Collaborator

pablobm commented Apr 18, 2023

Closing in favour of #2311

@pablobm pablobm closed this Apr 18, 2023
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.

8 participants