Skip to content
This repository has been archived by the owner on Apr 20, 2018. It is now read-only.

fix #307 - grunt-watch spawn:false compatibility fix #323

Conversation

vlajos
Copy link
Contributor

@vlajos vlajos commented Mar 21, 2014

if the newly generated part is already in the config file with the same destination update only the source part, instead of add the whole block again. This way the same targets won't be regenerated multiple times if usemin is called multiple times which can happen with grunt-watcher spawn:false mode.

@sindresorhus
Copy link
Member

Can you explain the fix and add a test?

@vlajos
Copy link
Contributor Author

vlajos commented Jul 17, 2014

Sorry for the late response.
I try to explain the bug this patch fixes:
If you run grunt-usemin under grunt-watcher with spawn:false, everytime the task is triggered it inherits the config settings from the previous run, so the
outCfg.files = _.union(origCfg.files, cfg.files);
line re-add the same "section" again.
So for example if you use watcher to trigger a build when a file change happens, the first run will be fine, but when the second change happens the generated config files hash will get again the same entries, so it will build everything 2 times, and the third run will build everything 3 times, and so on.
The change I implemented changes the behaviour to not add a new section if the dst file is already there, but overwrite it. And this way it will build everything only 1 times.

I hope this description helped, but let me know If there is any more questions.

I added a test. Probably it is possible to create a shorter but I hope this is more meaningful.

If you run the test for the non-patched version and you add this line before the deepEquals assertion in the new test, you will see what was the original problem:

console.log(config.concat.generated.files);

@vlajos
Copy link
Contributor Author

vlajos commented Jul 17, 2014

I cleaned a bit this pull request. (rebased a few times, and fixed the js linting errors.) I hope the rebases don't cause any trouble.

@vlajos
Copy link
Contributor Author

vlajos commented Feb 20, 2015

ping.

@stephanebachelier
Copy link
Collaborator

@vlajos can you add a comment in the source file explaining the reason of the done. The commit message might be sufficient.
If you can add a comment the test explaining that you explicitly call process 2 times.

You need to update your code as it violate the jscs rule "one space before opening curly brace". See build output

… file with the same destination update only the source part, instead of add the whole block again. This way the same targets wont be regenerated multiple times if usemin is called multiple times which can happen with grunt-watcher spawn:false mode
@vlajos vlajos force-pushed the fix-307-grunt-watch-spawn-false-compatibility-issue branch from 40ba284 to ab78ca9 Compare February 21, 2015 23:46
vlajos added a commit to vlajos/grunt-usemin that referenced this pull request Feb 21, 2015
@vlajos vlajos force-pushed the fix-307-grunt-watch-spawn-false-compatibility-issue branch from ab78ca9 to 5ef5c0d Compare February 21, 2015 23:50
vlajos added a commit to vlajos/grunt-usemin that referenced this pull request Feb 21, 2015
@vlajos vlajos force-pushed the fix-307-grunt-watch-spawn-false-compatibility-issue branch from 5ef5c0d to a84df9d Compare February 21, 2015 23:55
vlajos added a commit to vlajos/grunt-usemin that referenced this pull request Feb 21, 2015
@vlajos vlajos force-pushed the fix-307-grunt-watch-spawn-false-compatibility-issue branch from a84df9d to f9b193c Compare February 21, 2015 23:55
@vlajos
Copy link
Contributor Author

vlajos commented Feb 21, 2015

Done.

@stephanebachelier
Copy link
Collaborator

LGTM

CC @sindresorhus

marcelaraujo pushed a commit to marcelaraujo/grunt-usemin that referenced this pull request Mar 20, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants