-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(tools): do not invoke tasks if not necessary #1443
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { Task } from './task'; | ||
|
||
export abstract class AssetsTask extends Task { | ||
shallRun(files: String[]) { | ||
return files.reduce((a, f) => { | ||
return a || (!f.endsWith('.css') && !f.endsWith('.sass') && | ||
!f.endsWith('.scss') && !f.endsWith('.ts')); | ||
}, false); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { Task } from './task'; | ||
|
||
export abstract class CssTask extends Task { | ||
|
||
shallRun(files: String[]) { | ||
return files.some(f => | ||
f.endsWith('.css') || f.endsWith('.sass') || f.endsWith('.scss')); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,25 @@ | ||
import * as gulp from 'gulp'; | ||
import { join } from 'path'; | ||
|
||
import { AssetsTask } from '../assets_task'; | ||
import Config from '../../config'; | ||
|
||
/** | ||
* Executes the build process, copying the assets located in `src/client` over to the appropriate | ||
* `dist/dev` directory. | ||
*/ | ||
export = () => { | ||
let paths: string[] = [ | ||
join(Config.APP_SRC, '**'), | ||
'!' + join(Config.APP_SRC, '**', '*.ts'), | ||
'!' + join(Config.APP_SRC, '**', '*.scss'), | ||
'!' + join(Config.APP_SRC, '**', '*.sass') | ||
].concat(Config.TEMP_FILES.map((p) => { return '!' + p; })); | ||
export = | ||
class BuildAssetsTask extends AssetsTask { | ||
run() { | ||
let paths: string[] = [ | ||
join(Config.APP_SRC, '**'), | ||
'!' + join(Config.APP_SRC, '**', '*.ts'), | ||
'!' + join(Config.APP_SRC, '**', '*.scss'), | ||
'!' + join(Config.APP_SRC, '**', '*.sass') | ||
].concat(Config.TEMP_FILES.map((p) => { return '!' + p; })); | ||
|
||
return gulp.src(paths) | ||
.pipe(gulp.dest(Config.APP_DEST)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation problem |
||
} | ||
}; | ||
|
||
return gulp.src(paths) | ||
.pipe(gulp.dest(Config.APP_DEST)); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/** | ||
* Base class for all tasks. | ||
*/ | ||
export abstract class Task { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 overriding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
/** | ||
* Override this task if you want to implement some custom | ||
* task activation mechanism. By default each task will be always executed. | ||
* | ||
* @param {string[]} files A list of files changed since the previous watch. | ||
*/ | ||
shallRun(files: string[]): boolean { | ||
return true; | ||
} | ||
|
||
/** | ||
* Implements your task behavior. | ||
* | ||
* @param {any} done A function which should be activated once your task completes. | ||
* @return {ReadWriteStream | Promise<any> | void} This method can either return a promise | ||
* which should be resolved once your task execution completes, a stream | ||
* which should throw an end event once your task execution completes | ||
* or nothing in case you will manually invoke the `done` method. | ||
*/ | ||
abstract run(done?: any): any | Promise<any> | void; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { Task } from './task'; | ||
|
||
export abstract class TypeScriptTask extends Task { | ||
shallRun(files: String[]) { | ||
return files.reduce((a, f) => { | ||
return a || f.endsWith('.ts'); | ||
}, false); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,9 @@ import * as isstream from 'isstream'; | |
import { join } from 'path'; | ||
import * as tildify from 'tildify'; | ||
|
||
import { changeFileManager } from './code_change_tools'; | ||
import { Task } from '../../tasks/task'; | ||
|
||
/** | ||
* Loads the tasks within the given path. | ||
* @param {string} path - The path to load the tasks from. | ||
|
@@ -14,6 +17,33 @@ export function loadTasks(path: string): void { | |
readDir(path, taskname => registerTask(taskname, path)); | ||
} | ||
|
||
function normalizeTask(task: any, taskName: string) { | ||
if (task instanceof Task) { | ||
return task; | ||
} | ||
if (task.prototype && task.prototype instanceof Task) { | ||
return new task(); | ||
} | ||
if (typeof task === 'function') { | ||
return new class AnonTask extends Task { | ||
run(done: any) { | ||
if (task.length > 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. indentation for the run method seems to be wrong starting from here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 task(done); | ||
} | ||
|
||
const taskReturnedValue = task(); | ||
if (isstream(taskReturnedValue)) { | ||
return taskReturnedValue; | ||
} | ||
|
||
done(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ending here (indentation problem) |
||
} | ||
}; | ||
} | ||
throw new Error(taskName + ' should be instance of the class ' + | ||
'Task, a function or a class which extends Task.'); | ||
} | ||
|
||
/** | ||
* Registers the task by the given taskname and path. | ||
* @param {string} taskname - The name of the task. | ||
|
@@ -24,19 +54,13 @@ function registerTask(taskname: string, path: string): void { | |
util.log('Registering task', util.colors.yellow(tildify(TASK))); | ||
|
||
gulp.task(taskname, (done: any) => { | ||
const task = require(TASK); | ||
if (task.length > 0) { | ||
return task(done); | ||
} | ||
const task = normalizeTask(require(TASK), TASK); | ||
|
||
const taskReturnedValue = task(); | ||
if (isstream(taskReturnedValue)) { | ||
return taskReturnedValue; | ||
if (changeFileManager.pristine || task.shallRun(changeFileManager.lastChangedFiles)) { | ||
return task.run(done); | ||
} else { | ||
done(); | ||
} | ||
|
||
// TODO: add promise handling if needed at some point. | ||
|
||
done(); | ||
}); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import { join } from 'path'; | |
import * as runSequence from 'run-sequence'; | ||
|
||
import Config from '../../config'; | ||
import { changeFileManager } from './code_change_tools'; | ||
import { notifyLiveReload } from '../../utils'; | ||
|
||
const plugins = <any>gulpLoadPlugins(); | ||
|
@@ -17,8 +18,13 @@ export function watch(taskname: string) { | |
join(Config.APP_SRC,'**') | ||
].concat(Config.TEMP_FILES.map((p) => { return '!'+p; })); | ||
|
||
plugins.watch(paths, (e:any) => | ||
runSequence(taskname, () => notifyLiveReload(e)) | ||
); | ||
plugins.watch(paths, (e: any) => { | ||
changeFileManager.addFile(e.path); | ||
|
||
runSequence(taskname, () => { | ||
changeFileManager.clear(); | ||
notifyLiveReload(e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation problem |
||
}); | ||
}); | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation problem