Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Import errors when there both a .css and .scss file in the specified location. #1222

Open
dbatiste opened this issue Oct 27, 2015 · 27 comments

Comments

@dbatiste
Copy link

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.

@mgreter
Copy link
Contributor

mgreter commented Oct 27, 2015

This is intentional and equal to how ruby sass handles this case.

@austinsmorris
Copy link

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.

@xzyfer
Copy link
Contributor

xzyfer commented Oct 27, 2015

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.

@mgreter
Copy link
Contributor

mgreter commented Oct 27, 2015

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.

@xzyfer
Copy link
Contributor

xzyfer commented Oct 28, 2015

I agree, we should add the flag and have it disabled by default to match the Ruby behaviour.

@rankida
Copy link

rankida commented Oct 28, 2015

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!

@xzyfer
Copy link
Contributor

xzyfer commented Oct 28, 2015

I've created sass/libsass#1656 to track this fix in LibSass.

@hkassam
Copy link

hkassam commented Nov 3, 2015

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").

@chriseppstein
Copy link
Contributor

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.

@mgreter
Copy link
Contributor

mgreter commented Nov 3, 2015

@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.

@hkassam
Copy link

hkassam commented Nov 3, 2015

@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/)

@chriseppstein
Copy link
Contributor

@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.

@hkassam
Copy link

hkassam commented Nov 3, 2015

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.

@dbatiste
Copy link
Author

dbatiste commented Nov 3, 2015

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.

@xzyfer
Copy link
Contributor

xzyfer commented Nov 28, 2015

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.

@pleunv
Copy link

pleunv commented Jan 6, 2016

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.

@airtonix
Copy link

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.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 25, 2016

@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.

@baamenabar
Copy link

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.

@nschonni nschonni added this to the 4.0 milestone Nov 8, 2016
@mcropper
Copy link

mcropper commented Nov 8, 2017

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.

@d07RiV
Copy link

d07RiV commented Dec 18, 2017

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.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 19, 2017

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
Copy link
Contributor

@nex3 should this be added as an error to Ruby/Dart?

@xzyfer
Copy link
Contributor

xzyfer commented Dec 20, 2017

@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.

@nschonni
Copy link
Contributor

@xzyfer sorry, I know about the upstream issue, I was mixing this up with the issue about input and output directories overlapping

@AldoMX
Copy link

AldoMX commented May 12, 2018

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.

@d07RiV
Copy link

d07RiV commented Jun 2, 2018

@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.

@sass sass locked as resolved and limited conversation to collaborators Jun 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests