Skip to content

Destination CSS file is not updating using watchify #63

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

Closed
tinchoz49 opened this issue Nov 21, 2015 · 13 comments
Closed

Destination CSS file is not updating using watchify #63

tinchoz49 opened this issue Nov 21, 2015 · 13 comments

Comments

@tinchoz49
Copy link

Hi guys, working with your example: https://github.com/css-modules/browserify-demo
I ran into this problem: using watchify, the event emit 'update' works great but is not updating the dest css file.

run: npm start and edit a css file from the src folder.
The log gets info browserify: /index.js 682ms (bundle) but the dest css file is not changed.

@tinchoz49
Copy link
Author

I found that if you comment the line 47 and 48 of the file-system-loader works.
What is the objetive of create a token based on the filename? If is for caching I think is better create a checksum based on the content file.

@joshwnj
Copy link
Member

joshwnj commented Nov 23, 2015

Thanks for reporting this @tinchoz49 . It looks like a solution shouldn't be too tricky but there are a few different factors at play so we'll need to make sure other functionality isn't affected. Would you be interested in helping to put together a failing test case for this bug?

@justgook
Copy link

By commenting those lines it updates map, but not rebuild css-file

@markuplab
Copy link

Do you need help with this problem? I can try make fixes.

@justgook
Copy link

for me it is main stopper to use css-modules in my projects..

@joshwnj
Copy link
Member

joshwnj commented Nov 28, 2015

Thanks for the offer @markuplab - that'd be great, especially if you're able to help with writing a test case for this issue.

@joshwnj
Copy link
Member

joshwnj commented Nov 28, 2015

Status on this: I'm looking into a better way of walking the dependency tree, which I'm hoping will be the key to getting watchify working properly. But my time is limited so if anyone else cares to help that would be greatly appreciated :)

@joshwnj
Copy link
Member

joshwnj commented Nov 29, 2015

From early trials the solution appears to be working 🙆‍♀️ Need to check a few more edges and then will put together a release.

@markuplab
Copy link

Ohhh it's amazing :) Can you create PR?

@markuplab
Copy link

@joshwnj Hi, can you say, when release with this fix will be ready? We want start project with css-modules, but watchify problem block it, if you have troubles with time, we can fork module and fix it problem, or we can wait when you fix it?

@joshwnj
Copy link
Member

joshwnj commented Nov 30, 2015

@markuplab I've pushed #64, which still has a couple of bugs and failing tests (and I believe will need css-modules/css-modules-loader-core#62 to be merged in order to pass as well).

But even at this point it partly works :) And hopefully it gives you an idea of where it's heading, so please take a look and feel free to fix any bugs you see. I'm out of time today but hopefully will have time to wrap it up tomorrow.

@snikch
Copy link

snikch commented Nov 30, 2015

@joshwnj I've been watching this issue, and have tried out #64 and this seems to fix the issue for me.

@snikch
Copy link

snikch commented Nov 30, 2015

Hrm, I spoke too soon. It will handle a change which it hasn't seen before, but making a change then reverting it will not result in an update.

For example, running with a padding: 10px, then changing to padding: 60px will correctly rebuild the css, but changing back to padding: 10px will not. Updating to a new value, such as padding: 600px, will trigger an update.

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

No branches or pull requests

5 participants