Add an ability to build assets from Rails root folder

[mekhovov]

Add an ability to build assets from Rails root folder if it's css file is not found in app/assets/stylesheets/ directory.

It will allow us to include assets from engines, lib, vendor and any other non-standard folder.

I have tried similar PR #7 but it doesn't really work as it doesn't handle assets outside app/assets/stylesheets/ folder, which is enforced by the gem.

To respect backward compatibility - we will check if asset exist in app/assets/stylesheets/ folder, if it's not - we will set CSS_BUILD_PATH folder to Rail's root directory which will allow us to pass any custom folder for assets we need.

This is how we are using it in our app

# include assets from custom folders
assets_to_compile = [
  {
    source_folder: 'app/assets/stylesheets',
    files: ['**/*.scss', '**/*.css']
  },
  {
    source_folder: 'vendor/stylesheets',
    files: ['**/*.css']
  },
  {
    source_folder: 'lib/stylesheets',
    files: ['**/*.css']
  },
]

# include assets from engines, use custom `output_subfolder` subfolder inside `builds` directory
Dir["engines/**/stylesheets/*/"].each do |engne_path|
  assets_to_compile += [
    source_folder: engne_path.chomp('/'),
    output_subfolder: "#{engne_path.chomp('/').split('/').last}/",
    files: ['**/*.scss', '**/*.css']
  ]
end

dartsass_build_map = {}

# prepare map
assets_to_compile.each do |asset|
  asset[:files].each do |pattern|
    file_path_pattern = "#{asset[:source_folder]}/#{pattern}"
    Dir[file_path_pattern].each do |file_path|
      output_file_name = file_path.gsub("#{asset[:source_folder]}/", '').gsub(/\.scss$/, '.css')
      dartsass_file_map = { file_path => "#{asset[:output_subfolder]}#{output_file_name}" }
      dartsass_build_map.merge!(dartsass_file_map)
    end
  end
end

# add assets map to existing gem's API
Rails.application.config.dartsass.builds.merge!(dartsass_build_map)

Ideally, I would love to extend dartsass-rails gem API to support more abstracted build-map.
I was thinking about accepting source/output folders and file patterns similarly to example above ^^^

However, I haven't yet found found a good-enough-common pattern for it without introducing too much app-specific structure to the common gem...

Will be happy to brainstorm on that and improve implementation 🍺

I think current improvement makes sense, it's low risk and supports backward compatibility with existing API, expanding it for more complex rails apps like ours.

[mekhovov]

removed app/ folder from this example app/index.sass as it gives us wrong impression that we are starting from Rail's root folder (but we are technically enforcing prefix app/assets/stylesheets/ to each source file)

[anandbait-coupa]

Yeah! that's correct. When I was integrating even I thought the same and gave path accordingly. But later realised that it is from app/assets/stylesheets

[anandbait-coupa]

This looks like easy workaround for now.
Let me give a try to use assets's path instead of Rails.root. IMO that will be more inclined to Rails-way.
This will allow use to write less lengthy file path.
I will play around it.

[mekhovov]

awesome PR #11 was merged recently. so we have Rails.application.config.assets.paths in load-path now, which is great!

but it doesn't solve our use-case when we need to pre-compile (and watch) files located in non-standard assets folders outside of /app/assets/stylesheets

[aergonaut]

Instead of checking each path for existence in the CSS_LOAD_PATH, maybe we could go with the convention that a relative path is relative from CSS_LOAD_PATH, but an absolute path is used as-is. Then, Rails.root.join could be used to construct paths elsewhere inside the Rails root, while plain Strings could continue to reference files inside CSS_LOAD_PATH like they do today.

[mekhovov]

Instead of checking each path for existence in the CSS_LOAD_PATH, maybe we could go with the convention that a relative path is relative from CSS_LOAD_PATH, but an absolute path is used as-is. Then, Rails.root.join could be used to construct paths elsewhere inside the Rails root, while plain Strings could continue to reference files inside CSS_LOAD_PATH like they do today.

@aergonaut yeah, great idea! I was thinking about it as well :)
it should work fine for our use-case.

Rails.application.config.dartsass.builds = {
  "index.sass" => "app.css",  # relative to stylesheets path will be prefixed with CSS_LOAD_PATH
                              # generated full-path "/path-to-your-rails-app/app/assets/stylesheets/index.sass"
  "/lib/stylesheets/common.css" => "common.css",  # relative to rails path will be prefixed with Rails.root.join
                                                  # generated full-path "/path-to-your-rails-app/lib/stylesheets/common.css"
}

@dhh @vovimayhem @chipairon @aergonaut let me know if you like this convention, I'll be happy to make changes 🍻

[aergonaut]

Ah, sorry, that's not what I meant.

Let's say you configure builds like this:

Rails.application.config.dartsass.builds = {
  "app/index.sass" => "app.css",
  Rails.root.join("lib/stylesheets/common.sass") => "common.css"
}

The first entry is a relative path, so it would be prefixed with CSS_LOAD_PATH. This is the current behavior.

The second entry is an absolute path. (In this case, I constructed it using Rails.root.join, but that's unimportant. However the user chooses to construct the path shouldn't matter, all that should matter is that the path is absolute.) Because it is absolute, when compiling, we could skip any prefixing and use the path as-is. It's absolute anyway, so there is no ambiguity to where it points. This would be the new behavior.

We could test for absoluteness by converting the entry keys into Pathnames and calling absolute?. Pathname defines to_s to just emit the path, so I think this would be fine and wouldn't require a lot of changes to account for the entry keys now being a different kind of object.

Treating the entries like this would let people specify any file they want to be compiled. It's a potential footgun that they could specify files outside of the Rails.root, but maybe that's not a big problem, because it would cause all sorts of other issues otherwise, so I would expect people would quickly get the hint that that's not a good idea.

[mekhovov]

@aergonaut I'm fine with both options.
I thought it might not make sense to reference files outside or rails root folder anyway.
so I was thinking the / in the beginning of the file name could mean - start from rails's root folder, and then gem will construct an absolute path. so the rails app will not care of knowing that gem needs absolute path ;)

[chipairon]

I can't speak for the maintainers, but here is my suggestion:
I think the convention is to have the stylesheet entry points at app/assets/stylesheets/*.
Instead of Rails.root.join("lib/stylesheets/common.sass") => "common.css", I'd suggest adding an entry point like:

// file at app/asssets/stylesheets/common.scss
@import 'common';

Dartsass will be able to pick up the right load path, since lib/stylesheets is included by default in config.assets.paths.

For the case of entry points coming from engines, a similar approach could be taken.
Instead of:

# config/initializers/dartsass.rb
Rails.application.config.dartsass.builds = {
  ...,
  "engines/travel/app/assets/stylesheets/travel/main.scss" => "travel/main.css"
}

Add an entry point file app/assets/stylesheets/travel/main.scss:

@import 'travel/main';

And in the initializer:

# config/initializers/dartsass.rb
Rails.application.config.dartsass.builds = {
  ...,
  "travel/main.scss" => "travel/main.css"
}

While that approach could get the job done without any change to this gem, I agree it would be great to have a composable way to add entry points from engines via an initializer with this shape:

# engines/travel/config/initializers/dartsass.rb
Rails.application.config.dartsass.builds << { # NOTE: not valid syntax right now, as 'builds' is a Hash
  "travel/main.scss" => "travel/main.css"
}

That mechanism would remove the need to know (from a host app) how a particular engine is building their assets and keep up with changes, letting each engine add to the pipeline what it needs.

[elliotcm]

Hey folks, any progress on this PR?

[anandbait-coupa]

@elliotcm No. We dropped usage of dartsass-rails as of now. We will revisit that approach in future upgrades of our project

[mekhovov]

Hey folks, any progress on this PR?

hey @elliotcm could you explain the issue you are facing with?
let's see if we could find out solution for your issue and involve maintainers to brainstorm on it

[elliotcm]

@mekhovov Pretty much the same issue as mentioned above; working within an engine and not being able to add gem assets into the dartsass pipeline from within the gem, you currently have to get the top level app developer to add a top level (S)CSS file as a workaround:

1. https://github.com/SmartCasual/munificent-admin/blob/main/lib/munificent/admin/engine.rb#L17-L19
2. https://github.com/SmartCasual/munificent-admin/blob/main/test/dummy/app/assets/stylesheets/munificent-admin.scss
3. https://github.com/SmartCasual/munificent-admin/blob/main/app/assets/stylesheets/munificent/admin/application.scss

Ideally we'd want to skip step 2 so that the end user of the engine doesn't have to alter their app's assets until and unless they want to explicitly override them.

[anthony0030]

This would be very useful for me. I would like to build assets from /app/assets/fonts/. Are there any updates or an ETA for this?