sass / sassc-rails

Integrate SassC-Ruby with Rails!
MIT License
706 stars 104 forks source link

@import doesn't handle file extensions #123

Open roberts1000 opened 5 years ago

roberts1000 commented 5 years ago

Everything I'm about to explain, works fine with sass-rails 5.0.7, boostrap-sass 3.7.1 and Ruby 5.2.X. I have a Rails engine that provides prebuilt styles for a Rails app. My engine has a app/assets/stylesheets/main.scss.erb file that calls:

@import '<%= "#{MyGem.root}/app/assets/stylesheets/my_gem/bootstrap_variables" %>';

It's trying to load this file, which also exists inside the gem:

/home/dev/Projects/my_gem/app/assets/stylesheets/my_gem/bootstrap_variables.scss.erb

I get the following errors when using sassc-rails 2.1.1 and sass-rails 6.0.0.beta.3. (I also have bootstrap-sass 3.4.1 loaded, which recently changed from sass to sassc.)

# SassC::SyntaxError:
#   Error: File to import not found or unreadable: 
    /home/dev/Projects/my_gem/app/assets/stylesheets/my_gem/bootstrap_variables.

Issue #1: Something isn't aware that the file actually ends in .sccs.erb and fails to find the file.

If I change the @import statement and explicitly add .scss.erb, it does find the correct file, but it hits another error:

# SassC::SyntaxError:
#   Error: Invalid CSS after "<": expected 1 selector or at-rule, was "<%#****************"

Which raises a second issue:

Issue #2: Something isn't running the bootstrap_variables.scss.erb file through the ERB processor before the SASSC processor evaluates the file. The beginning of boostrap_variables.scss.erb file has:

<%#****************************************************************************************
BOOTSTRAP VARIABLE OVERRIDES

Use this file to ...etc...
******************************************************************************************%>

// COLORS
////////////////////////////////////////////////////////////////////////////////

$brand-primary: #A6C2E0;
$brand-info: #BFC9D4;

The odd thing, is this all starts from this @import statement, which is inside main.scss.erb file

@import '<%= "#{MyGem.root}/app/assets/stylesheets/my_gem/bootstrap_variables" %>';

So it's definitely processing the ERB at this point

roberts1000 commented 5 years ago

Some additional details:

A relative path and no extension works! It finds the bootstrap_variables.scss.erb and processes the ERB inside of it:

@import 'my_gem/bootstrap_variables';

Extensions lets the file be found, but an error is thrown parsing the ERB content inside of it. These forms all fail:

@import '<%= "my_gem/bootstrap_variables.scss.erb" %>';   //has ERB to dynamically generate path
@import 'my_gem/bootstrap_variables.scss.erb';  // relative path, but has extensions
@import '/home/dev/Projects/my_gem/app/assets/stylesheets/my_gem/bootstrap_variables.scss.erb'; // absolute path and extensions

A hard coded full path (i.e. removing the ERB for the @import line), but no extension, causes the file not found error:

@import '/home/dev/Projects/my_gem/app/assets/stylesheets/my_gem/bootstrap_variables';
bodnarbm commented 5 years ago

I've created PR #136 to address the first issue that @roberts1000 noted (the sassc-rails gem not detecting files imported with an absolute path that does not include the file extension). This appears to an an issue with the search paths generation code not handling the case where the path specified in the import might not be a path relative to one of the load paths for the assets but instead is absolute from the root of the computer. As note by this issue, this could be problematic when dealing with assets between gems that may reference their own assets or expected generated assets within the host Rails application via an absolute path.

However, this will not address the second issue noted (where if the extension is given the file does not go through preprocessing). If this is behavior intended to be supported, then it will require a separate change to detect the extension type of the imported file via the provided extension and preprocess that file as if the extension was left off. I'm happy to submit a PR for that change too.

bodnarbm commented 5 years ago

137 Should address the second issue noted, assuming that it is expected behavior. I believe it should be since non-erb files work as imports as is when they include their extensions in the import.