Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Imports don't seem to get recompiled #157

Closed
achalddave opened this issue Sep 26, 2013 · 19 comments
Closed

Imports don't seem to get recompiled #157

achalddave opened this issue Sep 26, 2013 · 19 comments

Comments

@achalddave
Copy link

If I have a file index.scss which has @import "common.scss", and change common.scss, index.scss doesn't seem to get recompiled.

The relevant code seems to be in the following locations:

checkImports is called here, and defined here.

It relies on imports[path], which only seems to be assigned here to an empty list. I can't seem to find any code that detects imported files -- am I missing something?

@andrew
Copy link
Contributor

andrew commented Oct 19, 2013

@achalddave Is this still happening in the latest version of node-sass, v0.6.7?

@achalddave
Copy link
Author

I'll try to test this as soon as I can, but in the current master I don't see imports[path] being assigned to anything but an empty list... am I missing something?

@achalddave
Copy link
Author

Does not work as of version 0.7.0-alpha. I can try to revert to 0.6.7 and try it later -- this is the version npm automatically updated to.

@andrew
Copy link
Contributor

andrew commented Oct 20, 2013

If it doesn't work in 0.7.0-alpha then I doubt it will work in 0.6.7 either, I'll try and take a look at this one this week.

@achalddave
Copy link
Author

Yeah, I assumed so as well. I looked at it a while back and couldn't figure it out. imports doesn't seem to be set anywhere as I mentioned before, and I couldn't even figure out where you could get the imported paths from the back end (specifically, in Render). I couldn't spend too much more time on it, though, so I may have missed something obvious.

@deanmao
Copy link
Contributor

deanmao commented Jan 3, 2014

How are you running the command? If you don't have a --watch on the file, it won't get recompiled. You can pass the force flag into the middleware if you don't want the middleware caching the previous set of results.

@achalddave
Copy link
Author

I recently had my laptop stolen so I'm not able to test this right now and get back to you. But I was using it as middleware (I will try to find the repo where I was using it and point it out when I'm on a computer). The issue was that it was caching when it shouldn't be. I did end up using the force flag, but it seems that if an imported file is changed, the middleware should recompile the original CSS file. In fact, it seems the code attempts to do this, but some bug renders it useless, as I pointed out in the original comment.

@deanmao
Copy link
Contributor

deanmao commented Jan 4, 2014

The code that reads the import file paths are actually inside of libsass. It's possible the bug is there, but I would've expected a long more people to have this problem if it were actually broken in the native library.

@achalddave
Copy link
Author

Oh, you're right, it's probably more likely that I read the code incorrectly; just pointing out what I thought I saw. Will take a closer look later; does it work for you? I.e. If you change an imported file, does it automatically recompile the file you're importing it from?

@deanmao
Copy link
Contributor

deanmao commented Jan 4, 2014

No, it doesn't. I have to manually watch all the files that I intend to change during development or else it won't recompile it for me automatically. Although node-sass "looks" like it will handle everything, it only passes all the parameters to the native library that does everything else. We tend to get bugs that should have be filed with libsass because most don't realize that node-sass is not the actual sass compiler.

@achalddave
Copy link
Author

Hm, okay. I guess I'll check with libsass then. The thing is, node-sass seems to do some prechecking about what has changed before passing it to libsass, and it seemed to me like these imported files were simply not repassed to libsass. At any rate, I know very little about either code base, just what I read into when I filed this issue, so you likely know more. If you're sure this is a libsass bug, feel free to close it and I can report it there.

@jhnns
Copy link
Contributor

jhnns commented Apr 20, 2014

I haven't read the code thoroughly but I guess this feature is not possible without my pull-request #290 since we need to know which files have been imported to watch them. But this feature can probably be implemented when #290 is finished.

@jkodroff
Copy link

@jhnns I'm having the same problem with node-sass 0.9.3. Has it been identified whether it's a node-sass issue or a libsass issue? Will this be fixed in version 0.10.0 (which looks like it'll include your PR referenced above)?

In the meanwhile, it looks like setting up a grunt task to watch all files is the way to go so I'm gonna try that.

@jhnns
Copy link
Contributor

jhnns commented Sep 1, 2014

I guess the relevant code for it is here. Are you running node-sass via the command line?

My pull-request has already been merged some time ago, but the cli watcher has not been updated. Currently it only watches the current file or the given directories. You could change the behavior to update the watched directories depending on the imported files.

@maximeaubaret
Copy link

👍 I have the same problem here.

@swervo
Copy link

swervo commented Oct 9, 2014

I've just encountered this problem too.
The CSS isn't recompiled if an imported SCSS file changes.
This seems to be the case both with files imported locally and from directories set using:

includePaths

I'm using sass.middleware

But, using

force: true

is a good workaround

@am11
Copy link
Contributor

am11 commented Dec 25, 2014

Hey guys, v2.0.0-beta is just released. Is it still happening?

FWIW, middleware function is removed from node-sass now (not even the deprecated warning would appear).

See release notes for breaking changes.

@am11
Copy link
Contributor

am11 commented Feb 12, 2015

npm install node-sass@2.0.0 is released!
Checkout the release notes: https://github.com/sass/node-sass/releases/tag/v2.0.0

@am11 am11 closed this as completed Feb 12, 2015
@jhnns
Copy link
Contributor

jhnns commented Feb 12, 2015

Yay! 🏁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants