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

Migration from sassc-rails workarounds #43

Open
nevans opened this issue Jul 8, 2023 · 14 comments
Open

Migration from sassc-rails workarounds #43

nevans opened this issue Jul 8, 2023 · 14 comments

Comments

@nevans
Copy link

nevans commented Jul 8, 2023

It would help enormously if we had better documentation on the incompatibilities with sassc-rails and known workarounds for them. I'm willing to create a README.md or docs/migrating.md PR, but I'd need guidance (and I can't promise when I'll get around to creating it).

Following are my initial thoughts start towards what might be needed in a docs/migrating.md, and the workarounds that I used. 🙂 You can maybe consider it a "rough draft". I would appreciate advice on my workarounds: Can they be simplified or improved? Are they factually inaccurate (I did test them all, but there might be something abnormal in my testing env)? Which ones deserve to be included in a documentation PR? What am I missing?

Glob imports

Imports need to be explicitly enumerated. A simple pattern is to convert all @import "foo/**/*"; glob imports into @import "foo"; and add a new foo/_index.scss file which contains explicit imports for all of the globbed files. If foo/_index.scss already exists, @import "foo/glob"; and foo/_glob.scss could be used instead.

This can be tedious and error-prone to do by hand, so it would be nice to add a find|awk bash one-liner to the documentation. Even nicer would be a dartsass:migrate-globs task.

Require directives

Although sass-rails and sassc-rails both process the sprockets require directives in .scss files, dartsass-rails does not. I found several different workarounds for this. The first two should work with sprockets or propshaft, but the others require sprockets.

Convert to @import

Fortunately, dartsass-rails adds all gem asset paths as load paths for dartsass. So, if the gem is only using basic css or scss, we can generally replace //= require "foo" with @import "foo";

Unfortunately, when the gems themselves rely on sprockets features, this won't work. For example: jquery-ui-rails uses sprockets require directives. And font-awesome-rails uses erb to process a .css.erb file. Other gems may have other incompatibilities.

Convert to node packages

In many cases, a node package exists for the library in question. In that case, the simplest approach may be to convert from the bundled gems to the node package. The @import statement may need to use the full path to the stylesheet, relative to node_modules. E.g: //= require "jquery-ui" might be converted to @import 'jquery-ui/dist/themes/base/jquery-ui.min';. Alternatively, the path to the node package's dist directory could be added to config.assets.paths.

However, in some cases, the node packages require non-trivial changes to the application code, or the gems are significantly simpler to use, or the gems come with additional functionality such as image assets and helper modules. In those cases, the next two workarounds will probably work.

Use a sprockets css entrypoint to require dartsass build output

Add a prefix or suffix to the entrypoint with the problematic require directives. Add this new file to the config.dartsass.builds hash. Remove the sprockets requires from the top, to avoid confusion (optional, they are comments so they're simply ignored).

Create a new .css file with the same base name as the original entrypoint (only changing the extension from .scss to .css). Add this file to either config.assets.precompile or manifest.js. Copy all of the original require directives into this file (nothing else). Add a require directive for the renamed .scss file at the bottom of this css file--or if require self was used, replace that. Once dartsass has run and written to app/assets/builds, the new sprockets entrypoint should be able to work just like before.

Use new entrypoints and new stylesheet_link_tags

Convert the = require that work into @import. The problematic imports can mostly be discovered by simply converting them all and then commenting out the ones that don't work until dartsass:build runs without errors.

  • For each problematic import:
    • add the built name to config.assets.precompile. Sprockets precompiles these entrypoints using the asset paths and manifests of all loaded rails engines (gems).
    • Above every stylesheet_link_tag for the existing entrypoint, add a stylesheet_link_tag for each of these new entrypoints.
  • CSS ordering is significant and this moves the sprockets processed stylesheets to first. If that breaks the styles, create more entrypoints in either dartsass or sprockets as needed.

Examples

The examples are based on the following hypothetical broken scss:

// app/assets/stylesheets/admin.scss
/*
 *= require "bootstrap"  
 *= require "bootstrap-responsive"
 *= require "font-awesome"  
 *= require "jquery-ui"
 *= require "something-else-etc-etc"  
 */

.my.awesome.scss.styles {}
.etc .etc .etc {}
# config/initializers/assets.rb
config.dartsass.builds = { "admin.scss" => "admin.css" }
<%# app/views/layouts/admin.html.erb %>
<%= stylesheet_link_tag "admin", media: "all" %>

Use a sprockets css entrypoint to require dartsass build output

The example could be converted to something like the following:

// app/assets/stylesheets/admin.css
/*
 *= require "bootstrap"  
 *= require "bootstrap-responsive"
 *= require "font-awesome"  
 *= require "jquery-ui"
 *= require "something-else-etc-etc"  
 *= require "admin-scss"  
 */
// EOF
// app/assets/stylesheets/admin-scss.scss
.my.awesome.scss.styles {}
.etc .etc .etc {}
# config/initializers/assets.rb
config.assets.precompile += %w[admin.scss]
config.dartsass.builds = { "admin-scss.scss" => "admin-scss.css" }

The erb would be unchanged.

Use new entrypoints and new stylesheet_link_tags

The example would be converted to something like the following:

// app/assets/stylesheets/base-layout.scss
@import "bootstrap";
@import "bootstrap-responsive";
// app/assets/stylesheets/admin.scss
@import "something-else-etc-etc";

.my.awesome.scss.styles {}
.etc .etc .etc {}
# config/initializers/assets.rb
config.assets.precompile += %w[
  font-awesome.css
  jquery-ui.css
]
config.dartsass.builds = {
  "base-layout" => "base-layout.css"
  "admin-scss.scss" => "admin-scss.css"
}
<%# app/views/layouts/admin.html.erb %>
<%= stylesheet_link_tag "base-layout", media: "all" %>
<%= stylesheet_link_tag "font-awesome", media: "all" %>
<%= stylesheet_link_tag "jquery-ui", media: "all" %>
<%= stylesheet_link_tag "admin", media: "all" %>
@sdhull
Copy link

sdhull commented Jul 15, 2023

Thanks for this—I was especially relieved to see that globs simply don't work (and it's not something I screwed up). I think migration guide and/or utilities would be awesome.

@caiwilliamson
Copy link

caiwilliamson commented Aug 23, 2023

@nevans Thank you so much for this! Your explanation really helped me to understand. I was having trouble with an old version of the font-awesome-rails gem and also discovered that dartsass-rails wont process the .css.erb file contained in there. My solution at first was to copy font-awesome.css.erb out of the gem and into app/assets/stylesheets/font-awesome.css (without the .erb extension) and replace the small amount of erb that wasn't really necessary anyway so that dartsass could compile it. That worked fine and we don't really care about keeping this old dep up to date, but your example workarounds are more elegant so I went with one of those. As @sdhull said, I was also relieved to see someone else had the same difficulties migrating from sassc-rails, and I think this guide is definitely useful! Glad I stumbled upon it.

@h0jeZvgoxFepBQ2C
Copy link

Can someone explain why globs are not working anymore? its really hard to upgrade and I dont understand why the old gem was deprecated, when the new gem doesn't fulfill many features which were used before.

What is the recommeded way to migrate f.e. the rails-jquery-ui gem?

@dgmstuart
Copy link

//= require "jquery-ui" might be converted to @import 'jquery-ui/dist/themes/base/jquery-ui.min.css';

Won't this be interpreted as a "plain CSS import"?

In addition to importing .sass and .scss files, Sass can import plain old .css files. The only rule is that the import must not explicitly include the .css extension, because that’s used to indicate a plain CSS @import.

https://sass-lang.com/documentation/at-rules/import/#importing-css

I'm trying to work out an issue where this results in a plain CSS import:

@import "accessible-autocomplete/dist/accessible-autocomplete.min.css";

...while this works fine locally, but fails when building on Heroku:

@import "accessible-autocomplete/dist/accessible-autocomplete.min";
Error: Can't find stylesheet to import.
          ╷
       10 │ @import "accessible-autocomplete/dist/accessible-autocomplete.min";

@dgmstuart
Copy link

OK, turns out @use is probably the way to go (it's apparently fine to use with plain CSS files: https://sass-lang.com/documentation/at-rules/use/#loading-css).

My issue was resolved by adding a nodejs buildpack before the Ruby buildpack (I don't know why this is required).

@nevans
Copy link
Author

nevans commented Nov 8, 2023

//= require "jquery-ui" might be converted to @import 'jquery-ui/dist/themes/base/jquery-ui.min.css';

Won't this be interpreted as a "plain CSS import"?

Yes, you're right. Sorry about that. As you noticed, it should be fine if you simply drop the ".css" extension from the import. In my app, I have several css files that are successfully imported from node_modules... but they are all missing the extension. I updated the issue text to match.

@use should probably work whether or not you include the extension, but it does have other minor differences.

I haven't used heroku in a long time, but I'm guessing that, without the nodejs buildpack, it doesn't actually install the node packages into the node_modules directory.

@dgmstuart
Copy link

@nevans thanks so much for the response.

I think it turned out to be something much more subtle:

I'm using jsbundling-rails and dartsass-rails, and both of them inject a build step as prerequisite of assets-precompile using .enhance:
https://github.com/rails/dartsass-rails/blob/main/lib/tasks/build.rake#L35
https://github.com/rails/jsbundling-rails/blob/main/lib/tasks/jsbundling/build.rake#L45

The issue is that the ordering of these builds isn't explicitly defined - I guess it might be based on the order that the gems are listed in the Gemfile?

In my setup, dartsass:build gets run first, before javascript dependencies have been installed, so it fails when it tries to reference a css file inside node_modules.

Adding the Node buildpack to run before the Ruby buildpack has the effect of installing dependencies first, so that they're available when dartsass runs, with the downside that jsbundling will build everything again (unless we configure it not to).

I'm not sure what either of these gems could do to ensure that they're run in the correct order 🤔.

@dgmstuart
Copy link

To be clear: I don't think this is a Heroku problem: I think in any build environment with both jsbundling-rails and dartsass-rails where CSS is referenced in node_modules, the build will either:

  1. Work fine, but only because the gems are in a specific order
  2. Fail because there isn't an explicit js install step (eg. yarn install) before assets:precompile
  3. Pass because there's an explicit js install step, BUT build the JS twice (unless SKIP_JS_BUILD is true)

@nevans
Copy link
Author

nevans commented Nov 8, 2023

@dgmstuart Ah, I see. That makes sense. I didn't actually think about jsbundling-rails keeping node_modules up-to-date (I don't currently use it).

The issue is that the ordering of these builds isn't explicitly defined

...

I'm not sure what either of these gems could do to ensure that they're run in the correct order

If I remember correctly, dartsass-rails doesn't actually care if you are running any javascript engine at all (neither node, nor yarn, bun, etc). So from dartsass-rails's perspective, there is no dependency. Therefore from rake's perspective, the order isn't defined. I've been getting along okay because my production image build script explicitly runs yarn prior to bin/rails dartsass:build. (Tangentially: I don't remember why my build script was written in bash rather than rake!)

But we can add our own deps to existing tasks if we want it to work reliably. Since you're using jsbundling-rails, maybe you could also try adding an explicit dependency of your own to a lib/tasks/assets.rake file? Maybe something like:

task "dartsass:build" => "javascript:install"

With that, you should (probably) be able to rely on rake to ensure that the JS deps are only installed once and your JS is only built once.

@dgmstuart
Copy link

@nevans

you could also try adding an explicit dependency of your own to a lib/tasks/assets.rake file? Maybe something like:

task "dartsass:build" => "javascript:install"

This worked! Thanks so much 🙏

dgmstuart/swing-out-london#276

It's hard to tell, but it does indeed look like this only results in the JS being installed once.

@dgm
Copy link

dgm commented Nov 10, 2023

I'm still struggling with this - I have font-awesome and active_scaffold both misbehaving due to scss.erb usage ... but an added wrinkle is that in my own scss files I have an @extend .fa call so the font-awesome files need to be available to import. I'm a bit lost at the moment.

@dgmstuart
Copy link

Hi @dgm - could you be more specific? What is failing, where and with what error/behaviour?

Looks like the old way of including font-awesome was this?

/*
 *= require font-awesome
 */

...so I guess this has become @import "font-awesome" after the migration?

@dgm
Copy link

dgm commented Nov 13, 2023

Yes, I have to use @import "font-awesome" because I'm later using @extend .fa; which needs font-awesome to be in the sass compile chain. It was complaining that the file wasn't found.

I was able to get things working using the dartsass-sprockets gem instead.

@mikevoets
Copy link

mikevoets commented Oct 14, 2024

We're still using Rails Sprockets and we didn't want to run dartsass:build before assets:precompile, because assets:precompile does this for us. To make Sprockets always find the by dartsass compiled CSS, we had to add this to our config/initializers/assets.rb:

# config/initializers/assets.rb

Rails.application.config.assets.configure do |env|
  env.prepend_path(Rails.root.join('app/assets/builds'))
end

The reason for this hook is that Sprockets loads the assets paths before dartsass-rails compiles the CSS. Therefore, when it runs the first time, it won't find CSS in the app/assets/builds folder and then Sprockets will try to compile the SCSS itself, and run into Cannot load such file -- sass. With this fix, it will discover the CSS built by dartsass and won't attempt to compile the SCSS.

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

No branches or pull requests

7 participants