feat(tools): do not invoke tasks if not necessary #1443
Conversation
| if (typeof task === 'function') { | ||
| return new class AnonTask extends Task { | ||
| run(done: any) { | ||
| if (task.length > 0) { |
There was a problem hiding this comment.
indentation for the run method seems to be wrong starting from here
There was a problem hiding this comment.
hm... here it looks good, not sure if it is a bug in the review view of github or if it is shown nicely here (the same for the other hints about indentation)
| return taskReturnedValue; | ||
| } | ||
|
|
||
| done(); |
There was a problem hiding this comment.
ending here (indentation problem)
| }; | ||
| } | ||
| throw new Error(`${taskName} should be instance of the class | ||
| Task, a function or a class which extends Task`); |
There was a problem hiding this comment.
indentation problem as well
|
|
||
| runSequence(taskname, () => { | ||
| changeFileManager.clear(); | ||
| notifyLiveReload(e); |
| export abstract class CssTask extends Task { | ||
|
|
||
| precondition(files: String[]) { | ||
| return files.reduce((a, f) => { |
There was a problem hiding this comment.
couldn't all those reduce calls be simplified with files.some(f => f.endsWith('.css') || ...)?
| .pipe(gulp.dest(Config.APP_DEST)); | ||
| }; | ||
| return gulp.src(paths) | ||
| .pipe(gulp.dest(Config.APP_DEST)); |
|
|
||
| precondition(files: String[]) { | ||
| return super.precondition(files) || files.reduce((a, f) => { | ||
| return a || f.endsWith('.html'); |
| precondition(files: String[]) { | ||
| return files.reduce((a, f) => { | ||
| return a || (!f.endsWith('.css') && !f.endsWith('.sass') && | ||
| !f.endsWith('.scss') && !f.endsWith('.ts')); |
There was a problem hiding this comment.
I would indent this line once more
| * | ||
| * @param {string[]} files A list of files changed since the previous watch. | ||
| */ | ||
| precondition(files: string[]): boolean { |
There was a problem hiding this comment.
I would name the method differently, something like shallRun, would make its usage clearer IMO
if (task.shallRun(changeFileManager.lastChangedFiles)) {
return task.run(done);
}
There was a problem hiding this comment.
Maybe it's too late for this one, but I'll follow NG2 pattern and use a canRun or something like that. Sorry, I was out today.
| /** | ||
| * Base class for all tasks. | ||
| */ | ||
| export abstract class Task { |
There was a problem hiding this comment.
You could provide another abstract class which uses a regular expression to test file endings. This way a subclass would only need to pass an array of file endings to this abstract class instead of overridingprecondition (shallRun respectively, see below)
There was a problem hiding this comment.
Yes, I considered this idea here. It might be a good idea but I'd prefer to keep the API minimalistic for now. Maybe in future will turn out that the most important precondition is not related to file type.
mgechev
force-pushed
the
task-perf
branch
2 times, most recently
from
e558d18
to
65945b8
Compare
|
@robstoll thanks for the review! It seems vim messed the indentation up during the rebase I did. |
In dev executes tasks only when a specific file type has changed. Fix #1432.
In dev executes tasks only when a specific file type has changed.
Fix #1432.