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

v4 watch now runs one time and no more in some cases #1624

Closed
rosskevin opened this issue Apr 30, 2016 · 11 comments
Closed

v4 watch now runs one time and no more in some cases #1624

rosskevin opened this issue Apr 30, 2016 · 11 comments

Comments

@rosskevin
Copy link

Commit: 6c03475

I apologize in advance for this terrible issue report, I've been chasing this for hours and I'm afraid I have little to share, but I hope in the end someone can point me in the general direction of my error.

I had no problems until I pulled the latest, I generally stay up to date. Both gulp and gulp-cli are the lates.

I note from the ^^^commit a few others have seen the same change break their code. It is likely to be improper use, but I'm not spotting it in my code.

I created a test case and have been unable to reproduce:
https://github.com/alienfast/gulp-pipeline/tree/master/test/watch-repeated

This test case works fine, both in the simple form and with the gulp-pipeline recipe form.

The case where it doesn't work is in when using an Aggregate, which does some task generation and surely this is the most likely place for my error.

What I can say, definitively, is that the file watcher is triggering and I receive those events, but inside glob-watcher #runComplete is never run again (after the first run).

Since #runComplete is never run, what error would that indicate in my use? Missing passing through the done?

@phated
Copy link
Member

phated commented Apr 30, 2016

Aggregate is really complex, but I'm guessing the problem is that you don't return the this.taskFn result at https://github.com/alienfast/gulp-pipeline/blob/master/src/aggregate.js#L123 which means returning a stream doesn't actually get passed to gulp.

@phated
Copy link
Member

phated commented Apr 30, 2016

This is essentially the same problem and solution as #1623 (comment)

The behavior was inconsistent between gulp.task and gulp.watch and by normalizing it, it has highlighted that many people were actually writing code that wasn't working correctly.

@rosskevin
Copy link
Author

Thanks @phated, I had tried the return there but not in a deliberate manner so I pulled it. I just re-checked it (and committed it), same behavior.

But I get what you are saying, I'll look to see if there is an opportunity to make it simpler. My other thought was a coding error in my parallel and series helpers, which are essential parts to using the Aggregate.

Thanks again for looking.

@rosskevin
Copy link
Author

@phated - would it be possible to implement a paranoid check in the code so that these types of user errors are obvious? I'm a fan of the break early and often, perhaps we could check return values and blow up if it isn't a stream, promise, etc?

@phated
Copy link
Member

phated commented Apr 30, 2016

We have a warning message when you run a task without watch but I don't know how to adapt it for watch. Afk right now, sorry I can't link the cli code

@rosskevin
Copy link
Author

rosskevin commented Apr 30, 2016

I may have found the issue:

The thing is, an Aggregate has n tasks, and the great thing about it is it registers watches for all of the tasks, and if any change, it runs the aggregate set.

You can see I don't return a value from watchFn, because I'm actually registering say 4 watches and don't know which to return or which is running.

Any thoughts on that?

@phated
Copy link
Member

phated commented Apr 30, 2016

Your watch function isn't async. Everything must be async in gulp 4. This is the behavior that wasn't normalized until now.

@rosskevin
Copy link
Author

I tried running multiple watches in a few ways within the task, no success. Ultimately I aggregated the globs for a single call to gulp.watch. While that isn't foolproof, it is much simpler.

Thank you

@phated
Copy link
Member

phated commented Apr 30, 2016

@rosskevin glad you got something working. To be clear, it is not a problem with multiple watches, but instead a problem with you not having an async watch task that proxied to your other tasks. When you wrap functions you should always call them with the same arguments as the wrapper and return the result from the wrapper.

@rosskevin
Copy link
Author

Agreed. I tried but failed miserably. I just bumped into another case where I want to do something similar.

How could you run 3 watches from a single registered task and have it return properly? The readme has a sample but I see it doesn't return either, does it?

function watch() {
  gulp.watch(paths.scripts.src, scripts);
  gulp.watch(paths.styles.src, styles);
}

@phated
Copy link
Member

phated commented May 2, 2016

You should just be able to receive and call the done callback. All the examples haven't been updated yet.

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

2 participants