Import errors when there both a .css and .scss file in the specified location.
dbatiste opened this issue · 27 comments
Given dependency path that has both:
foo.css
foo.scss
Attempting to import foo.scss in a dependent file:
@import 'foo';
results in an error:
{
"message": "It's not clear which file to import for '@import "foo"'.\nCandidates:\n foo.scss\n foo.css\nPlease delete or rename all but one of these files.\n",
"column": 1,
"line": 1,
"file": "application.scss",
"status": 1
}
This worked prior to the recent update. Is this change intentional, bug? Explicitly specifying the extension in the @import statement resolves the issue for a particular case, however this is breaking for many components.
This is intentional and equal to how ruby sass handles this case.
I am also getting this issue when there is two .scss files, but one is proceeded with an underscore. For example:
@import "path/to/foo";
And then in the /path/to/ directory, I have both foo.scss and _foo.scss.
This was working prior to 3.4.0.
There are two separate issues here. Each of the foo files contains only a comment with the filename.
First, error when an importing foo with both foo.scss and _foo.scss available.
$ ls -l
total 32
-rw-r--r-- 1 michael staff 13 27 Oct 23:36 _foo.scss
-rw-r--r-- 1 michael staff 10 27 Oct 23:36 foo.scss
$ echo "@import 'foo'" | sass -s
Error: It's not clear which file to import for '@import "foo"'.
Candidates:
_foo.scss
foo.scss
Please delete or rename all but one of these files.
on line 1 of standard input
Use --trace for backtrace.
Error: It's not clear which file to import for '@import \"foo\"'.
Candidates:
_foo.scss
foo.scss
Please delete or rename all but one of these files.
on line 1 of stdin
>> @import 'foo'
^
In this case we're doing exactly what Ruby Sass does. The old working behaviour was a bug that has been fixed.
First, error when an importing foo with both foo.scss and foo.css` available.
$ ls -l
total 32
-rw-r--r-- 1 michael staff 13 27 Oct 23:36 foo.css
-rw-r--r-- 1 michael staff 10 27 Oct 23:36 foo.scss
$ echo "@import 'foo'" | sass -s
DEPRECATION WARNING: Importing from the current working directory will not be
automatic in future versions of Sass. To avoid future errors, you can add it
to your environment explicitly by setting `SASS_PATH=.`, by using the -I command
line option, or by changing your Sass configuration options.
/* foo.scss */
$ echo "@import 'foo'" | ./node_modules/.bin/node-sass
Error: It's not clear which file to import for '@import \"foo\"'.
Candidates:
foo.scss
foo.css
Please delete or rename all but one of these files.
on line 1 of stdin
>> @import 'foo'
^
We are incorrect here. This is a bug we need to fix.
This is the case because libsass allows embeddable CSS partials by default. If you enable this for ruby sass, you will get the same error too. Only improvement I would see is to add a config option to enable/disable this feature, as I have suggested before.
I agree, we should add the flag and have it disabled by default to match the Ruby behaviour.
This has caused us great issues in our project and have had to revert back to 3.3.0 in the mean time.
I would vote for the feature to be turned off/on, though I am surprised that the default is on. Indeed I am surprised that when given the option of filename.scss and filename.css the .scss doesn't simply win by default.
Must say, good work and thanks to the project team - node-sass is great!
I've created sass/libsass#1656 to track this fix in LibSass.
I would actually suggest that, since this is a breaking change with the way it was working before, it required a major version bump (as this change breaks every project that references node-sass in their package.json as "^3.x.y").
Sass only gives this error if there are sass files (.sass, .scss) with the same basename after removing the partial prefix. It is common practice for people to generate their css files into the same folder as their sass files (I wouldn't do this, but many do).
In terms of when node-sass should bump major versions I would argue that any time node-sass makes a breaking change to match ruby sass's behavior, this is simply a bug fix and does not warrant a major number bump.
@chriseppstein FYI: sass behaves the same if the "option" super.merge('css' => :scss) is set ... and we only see this regression because we have added the error message for unambiguous imports.
@chriseppstein the problem with that is, anyone using node-sass has been using it a specific way and expects it to work that way (regardless of whether it works the same as ruby-sass or not). Changing this behaviour in a breaking change means EVERY project utilizing this module will break. If node-sass is using semantic versioning (which it seems to be), breaking changes require a major version bump (http://semver.org/)
@hkassam I know how semver works. But it will be a huge pain in the ass from a marketing and usability perspective if node-sass and ruby-sass are feature compatible and not using the same major/minor number for language syntax purposes. Also: as far as breaking changes go, this one is super easy to work around.
@mgreter that is not a default setting for Ruby Sass.
Ok, so I understand the headache of having node-sass and node-ruby being feature compatible being extremely desirable. I'm just saying that this change was breaking and should have updated the major version so as not to break ALL of the projects that are utilizing node-sass. An assumption that npm seems to use is that all node packages utilize semver. I would suggest, as it doesn't seem node-sass uses semver (which is fine), that a comment is made in the readme to indicate this.
From what I understand, and given @xzyfer 's comment above, it seems that this should have been a deprecation warning. This would also have given folks some time to make their includes more explicit before encountering build errors such as this.
When people use semver, they are trusting that breaking changes will not be introduced without an appropriate version change. Semver provides benefits to both providers and consumers - one of those benefits is that the dev community shares the testing, however when consumers can't trust revisions and minor version bumps, then they start opting out and pegging to specific versions, and this makes me sad.
It doesn't really matter how easy the work-around is. I appreciate the desire to keep the versions the same - perhaps that's an indication they shouldn't be maintained independently.
This was a breaking change that was not caught by either our tests or LibSass.
We will fix LibSass to match the Ruby Sass behaviour sass/libsass#1656 (comment)
Sass 4.0 will introduce a module system which will give the user more flexibility with their imports.
Would it not make more sense to default to the scss file when both a scss and css file are available, perhaps with an optional warning that a file conflict exists? This would allow you to use the new css @imports while simultaneously avoiding having to change most of your existing @import statements.
We have multiple themes importing a base scss file and several npm components in order to recompile it with extensions and updated variables. All of these imported files get compiled individually as well for use in projects that do not use Sass. Consequently all of these now throw errors if we update node-sass and would require us to change dozens of @import statements across multiple projects. Having a mix of @imports that contain extensions and others that do not is also pretty confusing and not exactly consistent behavior.
seriously, there is a vast and immense chasm between the number of people who care about the vanity of a version number and those who just don't want (or can't budget time) to fix projects (now broken) because of a vanity number.
@airtonix it appears you have not fully read the issue. This is a bug in a dependency that was not caught by their tests. Node Sass is powerless here. This is an uncommon edge case. The problem should be fixed the upcoming 3.5.0 beta.
I have no idea what your complaint about vanity numbers is.
In the future please ask yourself whether your comment adds value to problem at hand. If you're looking for software free of bugs I wish you luck in your search.
What we ended up doing was:
Find and replace in the whole project using the following pattern.
@import ('|")((?!compass).+)(?<!scss)('|")
the replace with
@import$1$2.scss$3
We have literally hundreds of imports, and at least 50 had this problem.
I hope this helps someone looking for this error.
So what's the fix for this? If I'm explicit in the @import another error (EISDIR) seems to arise with the regards to the mixins folder.
Doesn't this completely defeat the purpose of having an extension-less version of @import? Since an .scss file will usually be accompanied by a (generated) .css file, you will never be able to import it without explicitly specifying the extension. This sounds like a bug to me, especially since the error message says:
Please delete or rename all but one of these files.
Sure.. I'll delete one, only to have it reappear next time I update the other.
Since an .scss file will usually be accompanied by a (generated) .css file
Output to the input directory is not support. We suggest separating input and output i.e. src and dist - since only input files should be committed.
@nschonni this is a known bug in LibSass see sass/libsass#754 for background. It's a feature/bug I've been trying to back out a couple years now. The consensus atm is to leave the broken behaviour in place until we have first class CSS import support via Sass 4 modules.
@xzyfer sorry, I know about the upstream issue, I was mixing this up with the issue about input and output directories overlapping
Hello, the recommended way to use sass by create-react-app is to overlap source and destination directories, which eventually results in this issue.
I understand that keeping backwards-compatibility with the current behavior is your top priority, but please add a command line argument to override it, that would only require a feature number bump in semver.
@AldoMX this is how I encountered this issue.
I've switched to using custom-react-scripts, which is a mod over CRA that includes out of the box support for LESS/SASS import without creating unnecessary files.