include_paths handling

[rerion]

ORIGINAL ISSUE DESCRIPTION:

Could multi_sass_binary rule also support include_paths?

Usage scenario: Non-bazel [angular builder](https://angular.io/guide/workspace-config#additional-build-and-test- options) supports passing includePaths to stylePreprocessorOptions.

Bazel builders use multi_sass_binary to compile component sass styles. Compiling existing angular codebase, without refactoring import paths in components is impossible.

[rerion]

Here are my thoughts on include_paths attribute.

1. Doesn't it break hermeticity? Consider directory variables containing _variables.scss. global_stylesheet.scss contains @import 'variables'; Will Bazel be aware that outputs of such rule could change if content of _variables changed?

   sass_binary(
   name = "global_stylesheet",
   src = 'global_stylesheet.scss',
   include_paths = ['variables'],
   output_name = "global_stylesheet.css",
   )

2. It's common in many frontend component based frameworks (ie Angular), to have many style files living beside logic/template code inside some directory structure. In these files there may be imports to variables or theme files which are assumed to be found in global scope. To configure global scope in sass --load-path flag should be used.
   The problem here is that for compiling multiple files multi_sass_binary rule should be used, but it doesn't expose this configuration (as sass_binary does).

3. Proposed changes: to declare some sass files there already exists a rule - sass_library. Both multi_sass_binary and sass_binary would support attribute global_deps, which would take list of SassInfo, and make all files available for import in global scope (using --load-path and some ugly copying under the hood). include_paths attribute would be dropped, the only way to modify global_scope would be to explicitly say which dependencies should be found there.
   The advantage is, all dependencies would have to be explicitly passed, bazel fashion.

This issue is in angular is relevant: https://github.com/angular/angular/issues/31362

(@nex3) interested in this?

[nex3]

@alexeagle Do you have thoughts here? I'm not sure exactly where the right place to draw the line between making multi_sass_binary fully-featured and just encouraging users to use sass_binary directly for more advanced use-cases.

[rerion]

@nex3 , @alexeagle is there any progress on this?

this is blocking adoption of bazel in angular applications using sass, here's relevant ticket: https://github.com/angular/angular/issues/31362

I would be happy to submit appropriate patches, when resolution to this problem is agreed upon.

[alexeagle]

we should get @kyliau to weigh in, he added multi_sass_binary and knows the most about Angular CLI feature parity

[kyliau]

For more advanced use cases, sass_library + sass_binary is always the preferred approach. This should not require the users to refactor the import paths in components. If it does, then we need to fix it.

I'm mainly concerned about (1) - that using include_paths breaks hermeticity guarantee of Bazel. In your proposal (3), if multi_sass_binary supports deps that provide SassInfo, wouldn't it play the same role as sass_binary then?

If this issue pertains solely to @angular/bazel builder, then perhaps it's better to fix the builder than the underlying rule.

[rerion]

@kyliau (1) I'd say it potentially makes all sass files under these paths dependencies. https://sass-lang.com/documentation/at-rules/import#load-paths That's why I'd opt for passing those explicitly. (3) sass_binary flattens input into one output, multi_sass_binary declares output .css file for each direct .sccs input. That's usually what you want from sass, the executable itself supports dirTree -> dirTree pattern. https://sass-lang.com/documentation/cli/dart-sass#many-to-many-mode