AlchemyCMS / alchemy_cms

Alchemy is the Open Source Rails CMS framework for the component based web that can be used as classic server side rendered or headless CMS.
https://www.alchemy-cms.com
BSD 3-Clause "New" or "Revised" License
817 stars 312 forks source link

Precompile CSS files into Gem #2886

Closed tvdeyen closed 1 week ago

tvdeyen commented 1 month ago

What is this pull request for?

Check in compiled CSS in the gem and do not compile the Sass assets within the context of the host app anymore.

Notable changes

Removes the sassc-rails dependency 🥳

This also removes the ability to add additional .scss files into the admin css bundle, as well as to overwrite Sass variables.

But this is probably a rare use case anyway and the pros outweigh the cons here.

TODO

Checklist

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.96%. Comparing base (cb5841d) to head (e0102ae).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2886 +/- ## ======================================= Coverage 95.96% 95.96% ======================================= Files 232 232 Lines 6272 6272 ======================================= Hits 6019 6019 Misses 253 253 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

afdev82 commented 1 week ago

Is there something that I could do to help this PR to be merged? I am currently switching my application too to dartsass-rails, but because AlchemyCMS still requires sassc-rails, I can't. The asset pipeline still tries to compile my already compiled builds (and it throws errors).

tvdeyen commented 1 week ago

Is there something that I could do to help this PR to be merged? I am currently switching my application too to dartsass-rails, but because AlchemyCMS still requires sassc-rails, I can't. The asset pipeline still tries to compile my already compiled builds (and it throws errors).

@afdev82 We still need to decide on how we build and ship the compiled assets w/o using the host app for compilation. If you have any idea I would love hear from it

tvdeyen commented 1 week ago

@afdev82 so, I took some time today to rebase this with latest main. Local testing looks promising. Do you mind to test this branch in your app and provide feedback? Thanks

afdev82 commented 1 week ago

Hi, thank you. I will try this branch today. My first idea was to precompile the admin assets in one css and provide it with the gem, exactly what you are doing now I see.

This also removes the ability to add additional .scss files into the admin css bundle, as well as to overwrite Sass variables. But this is probably a rare use case anyway and the pros outweigh the cons here.

Yes, that's exactly what I am doing in my app 😆 I have the all.css manifest file in the vendor folder, that includes alchemy/admin and a custom file. Now I could simply append a stylesheet_link_tag with the custom file, it's quite the same.

tvdeyen commented 1 week ago

Hi, thank you. I will try this branch today. My first idea was to precompile the admin assets in one css and provide it with the gem, exactly what you are doing now I see.

This also removes the ability to add additional .scss files into the admin css bundle, as well as to overwrite Sass variables. But this is probably a rare use case anyway and the pros outweigh the cons here.

Yes, that's exactly what I am doing in my app 😆 I have the all.css manifest file in the vendor folder, that includes alchemy/admin and a custom file. Now I could simply append a stylesheet_link_tag with the custom file, it's quite the same.

Kind of, but you would need to inject that stylesheet_link_tag into the admin.html.erb file with either a content_for or something like Deface. Maybe we can provide something more robust, but this can be tackled later, I guess.

Thanks for checking out.

afdev82 commented 1 week ago

Yes, it's not a big deal I think. If I want to overwrite a style in all views, I could also overwrite the admin layout to include the file there. With updates it's not ideal, but I don't think there will be much going on in the admin layout file. At the moment I am playing around in the admin interface and it seems all good 👍

tvdeyen commented 1 week ago

@afdev82 just merged into main. Make sure to update your Gemfile