From 865ab8ffddbc937ea1a93a01b448815bd5b01bd6 Mon Sep 17 00:00:00 2001 From: Mike Pham Date: Sun, 26 Nov 2017 21:23:30 -0500 Subject: [PATCH 1/2] feat: changing project loading to only happen once per execution --- src/Chest.ts | 27 +++++++++++---------------- src/Core/Actions/Packages.ts | 16 +++++----------- src/Core/Actions/Typings.spec.ts | 24 +++++++++++++++--------- src/Core/Actions/Typings.ts | 17 ++++++++--------- src/Core/Actions/Yarn.ts | 12 ++++++++---- src/Core/Files.ts | 29 +++++++++++++---------------- src/Core/Interfaces/Updater.ts | 5 +---- src/Core/Interfaces/UpdaterType.ts | 4 ---- src/Core/Interfaces/index.ts | 1 - src/Core/Registry.spec.ts | 9 ++------- src/Core/Registry.ts | 5 ----- src/Core/UpdateScript.spec.ts | 11 +++-------- src/Core/UpdateScript.ts | 18 +++++++----------- 13 files changed, 73 insertions(+), 105 deletions(-) delete mode 100644 src/Core/Interfaces/UpdaterType.ts diff --git a/src/Chest.ts b/src/Chest.ts index bf9b022..ee165b3 100644 --- a/src/Chest.ts +++ b/src/Chest.ts @@ -1,24 +1,19 @@ -import { Log, Logger, Project, Registry, UpdaterType } from './Core' +import { Log, Logger, Project, Registry } from './Core' export class Chest { private static readonly log: Log = Logger('chest') - public static async run(root: string, ...args: string[]): Promise { - Chest.log.debug('run', root, ...args) - - const project = await Project.load(root) + public static run(cwd: string, ...args: string[]): Promise { + Chest.log.debug('run', cwd, args) const updaters = Registry.all() - Object.keys(updaters) - .filter(key => args.some(arg => arg.toLowerCase() === key.toLowerCase())) - .map(async name => { - const updater = updaters[name] - - if (updater.type === UpdaterType.Root) { - await updater.exec(root) - } else { - await Promise.all(project.children.map(child => updater.workspace(child))) - } - }) + return Project.load(cwd) + .then(project => Promise.all( + Object.keys(updaters) + .filter(scriptname => args.some(arg => arg.toLowerCase() === scriptname.toLowerCase())) + .map(scriptname => updaters[scriptname]) + .map(script => script.exec(project)) + )) + .then(() => Chest.log.done('done', cwd)) } } diff --git a/src/Core/Actions/Packages.ts b/src/Core/Actions/Packages.ts index 96eb375..9af6e89 100644 --- a/src/Core/Actions/Packages.ts +++ b/src/Core/Actions/Packages.ts @@ -1,6 +1,6 @@ import * as path from 'path' -import { Files, NPM, Project, Registry, UpdateScript, UpdaterType } from '../index' +import { Files, NPM, Project, Registry, UpdateScript } from '../index' /* * Propogates changes from the root package.json to child @@ -10,10 +10,10 @@ class Script extends UpdateScript { public static readonly Name: string = Files.extensionless(__filename) constructor() { - super(Script.Name, UpdaterType.Projects) + super(Script.Name) } - public async workspace(project: Project): Promise { + protected async workspace(project: Project): Promise { if (project.owner) { const source = await project.owner.npm const target = await project.npm @@ -26,14 +26,8 @@ class Script extends UpdateScript { target.repository = source.repository const filename = path.join(project.path, 'package.json') - - if (this.testing) { - /* istanbul ignore next */ - this.log.task('workspace', filename, JSON.stringify(target, null, 2)) - } else { - await Files.save(filename, target) - this.log.task('workspace', filename) - } + await Files.save(filename, target) + this.log.task('workspace', filename) } return project diff --git a/src/Core/Actions/Typings.spec.ts b/src/Core/Actions/Typings.spec.ts index e738118..8ef2e7a 100644 --- a/src/Core/Actions/Typings.spec.ts +++ b/src/Core/Actions/Typings.spec.ts @@ -1,7 +1,9 @@ import 'mocha' import { expect } from 'chai' -import { Files, Registry, TypeScriptOptions } from '../index' +import { Files, Project, Registry, TypeScriptOptions } from '../index' + +const TIMEOUT = 5000 const testables = Files.join(process.cwd(), 'testables') const single = Files.join(testables, 'single') @@ -11,16 +13,20 @@ describe('when collecting type declarations', () => { it('should execute single', () => { const script = Registry.get('typings') - return script.exec(single) - .then(project => project.json('tsconfig.json')) - .then(json => expect(json.compilerOptions.types).to.contain('chalk')) - }) + return Project.load(single).then(project => + script.exec(project) + .then(project => project.json('tsconfig.json')) + .then(json => expect(json.compilerOptions.types).to.contain('chalk')) + ) + }).timeout(TIMEOUT) it('should execute workspaces', () => { const script = Registry.get('typings') - return script.exec(workspaces) - .then(project => project.json('tsconfig.json')) - .then(json => expect(json.compilerOptions.types).to.contain('chalk')) - }) + return Project.load(workspaces).then(project => + script.exec(project) + .then(project => project.json('tsconfig.json')) + .then(json => expect(json.compilerOptions.types).to.contain('chalk')) + ) + }).timeout(TIMEOUT) }) diff --git a/src/Core/Actions/Typings.ts b/src/Core/Actions/Typings.ts index 145a007..6f568ab 100644 --- a/src/Core/Actions/Typings.ts +++ b/src/Core/Actions/Typings.ts @@ -1,4 +1,4 @@ -import { Files, NPM, Project, Registry, TypeScriptOptions, UpdateScript, UpdaterType } from '../index' +import { Files, NPM, Project, Registry, TypeScriptOptions, UpdateScript } from '../index' /* * Updates the "types" property of "tsconfig.json" files by @@ -8,16 +8,15 @@ class Script extends UpdateScript { public static Name = Files.extensionless(__filename) constructor() { - super(Script.Name, UpdaterType.Root) + super(Script.Name) } - public exec(rootpath: string): Promise { - return Project.load(rootpath) - .then(project => - this.declarations([project, ...project.children]) - .then(typings => this.typings(project, typings)) - .then(() => project) - ) + public exec(project: Project): Promise { + return super.exec(project).then(project => + this.declarations([project, ...project.children]) + .then(typings => this.typings(project, typings)) + .then(() => project) + ) } private declarations(projects: Project[]): Promise { diff --git a/src/Core/Actions/Yarn.ts b/src/Core/Actions/Yarn.ts index c0870d2..e0d596d 100644 --- a/src/Core/Actions/Yarn.ts +++ b/src/Core/Actions/Yarn.ts @@ -1,4 +1,3 @@ -import { UpdaterType } from '../Interfaces' import { Project } from '../Project' import { Registry } from '../Registry' import { UpdateScript } from '../UpdateScript' @@ -7,11 +6,16 @@ export class Yarn extends UpdateScript { public static readonly Name: string = 'yarn' constructor() { - super(Yarn.Name, UpdaterType.Root) + super(Yarn.Name) } - public exec(rootpath: string): Promise { - return Project.load(rootpath).then(project => this.run(project, 'yarn').then(() => project)) + public exec(project: Project): Promise { + return super.exec(project) + .then(project => this.run(project, 'yarn').then(() => project)) + } + + protected workspace(project: Project): Promise { + return this.run(project, 'yarn').then(() => project) } } diff --git a/src/Core/Files.ts b/src/Core/Files.ts index 5811c84..ac3d458 100644 --- a/src/Core/Files.ts +++ b/src/Core/Files.ts @@ -50,13 +50,12 @@ class InternalFiles { return path.join(...args) } - public async json(filepath: string): Promise { - if (await this.exists(filepath)) { - const buffer = await this.readfile(filepath) - return JSON.parse(buffer.toString()) - } - - throw new Error(`requested file ${filepath} does not exist`) + public json(filepath: string): Promise { + return this.exists(filepath).then(exists => { + return exists + ? this.readfile(filepath).then(buffer => JSON.parse(buffer.toString())) + : Promise.reject(new Error(`requested file ${filepath} does not exist`)) + }) } public readfile(filepath: string): Promise { @@ -72,14 +71,12 @@ class InternalFiles { }) } - public async listdirs(filepath: string): Promise { - const stats = await this.statfiles(filepath) - return stats.filter(stat => stat.dir).map(stat => stat.filename) + public listdirs(filepath: string): Promise { + return this.statfiles(filepath).then(stats => stats.filter(stat => stat.dir).map(stat => stat.filename)) } - public async listfiles(filepath: string): Promise { - const stats = await this.statfiles(filepath) - return stats.filter(stat => stat.file).map(stat => stat.filename) + public listfiles(filepath: string): Promise { + return this.statfiles(filepath).then(stats => stats.filter(stat => stat.file).map(stat => stat.filename)) } public mkdir(filepath: string): Promise { @@ -94,11 +91,11 @@ class InternalFiles { }) } - public async save(filepath: string, data: T): Promise { - await this.writefile(filepath, JSON.stringify(data, null, 2)) + public save(filepath: string, data: T): Promise { + return this.writefile(filepath, JSON.stringify(data, null, 2)) } - public async statfile(filepath: string): Promise { + public statfile(filepath: string): Promise { return new Promise((resolve, reject) => { fs.stat(filepath, (error: NodeJS.ErrnoException, stats: fs.Stats) => { if (error) { diff --git a/src/Core/Interfaces/Updater.ts b/src/Core/Interfaces/Updater.ts index 05ce284..c5ec49f 100644 --- a/src/Core/Interfaces/Updater.ts +++ b/src/Core/Interfaces/Updater.ts @@ -1,11 +1,8 @@ -import { UpdaterType } from './UpdaterType' import { Project } from '../Project' export interface Updater { name: string - type: UpdaterType - exec(rootpath: string): Promise - workspace(project: Project): Promise + exec(project: Project): Promise } export type Updaters = { diff --git a/src/Core/Interfaces/UpdaterType.ts b/src/Core/Interfaces/UpdaterType.ts deleted file mode 100644 index 524670b..0000000 --- a/src/Core/Interfaces/UpdaterType.ts +++ /dev/null @@ -1,4 +0,0 @@ -export enum UpdaterType { - Root = 'root', - Projects = 'projects', -} diff --git a/src/Core/Interfaces/index.ts b/src/Core/Interfaces/index.ts index 74d97b0..f38904e 100644 --- a/src/Core/Interfaces/index.ts +++ b/src/Core/Interfaces/index.ts @@ -2,4 +2,3 @@ export * from './Dictionary' export * from './NPM' export * from './TypeScriptOptions' export * from './Updater' -export * from './UpdaterType' diff --git a/src/Core/Registry.spec.ts b/src/Core/Registry.spec.ts index 6353616..8e02058 100644 --- a/src/Core/Registry.spec.ts +++ b/src/Core/Registry.spec.ts @@ -3,7 +3,6 @@ import 'mocha' import { expect } from 'chai' import { Files } from './Files' -import { UpdaterType } from './Interfaces' import { Registry } from './Registry' import { UpdateScript } from './UpdateScript' @@ -13,13 +12,13 @@ const ProjectsScriptName = `${ScriptName}-projects` class DoesNothingGoesNowhereRoot extends UpdateScript { constructor() { - super(RootScriptName, UpdaterType.Root) + super(RootScriptName) } } class DoesNothingGoesNowhereProjects extends UpdateScript { constructor() { - super(ProjectsScriptName, UpdaterType.Projects) + super(ProjectsScriptName) } } @@ -40,8 +39,4 @@ describe('when using the script registry', () => { expect(() => Registry.get('invalid')).to.throw() }) - it('should throw error when given unregistered scripts', () => { - expect(() => Registry.execute(process.cwd(), 'invalid')).to.throw() - }) - }) diff --git a/src/Core/Registry.ts b/src/Core/Registry.ts index 74873cb..a35fde9 100644 --- a/src/Core/Registry.ts +++ b/src/Core/Registry.ts @@ -1,5 +1,4 @@ import { Dictionary, Updater, Updaters } from './Interfaces' -import { Project } from './Project' export class Registry { private static readonly registrations: Updaters = {} @@ -16,10 +15,6 @@ export class Registry { return !!this.registrations[name.toLowerCase()] } - public static execute(root: string, ...args: string[]): Promise { - return Promise.all(args.map(arg => arg.toLowerCase()).map(name => this.registrations[name].exec(root))) - } - public static get(name: string): Updater { const key = name.toLowerCase() diff --git a/src/Core/UpdateScript.spec.ts b/src/Core/UpdateScript.spec.ts index 66cdb14..d3ddec2 100644 --- a/src/Core/UpdateScript.spec.ts +++ b/src/Core/UpdateScript.spec.ts @@ -2,7 +2,7 @@ import 'mocha' import { expect } from 'chai' -import { Files, Project, Registry, UpdateScript, UpdaterType } from './index' +import { Files, Project, Registry, UpdateScript } from './index' const NullScriptName = 'null-update' const NullScriptsName = 'null-updates' @@ -12,13 +12,13 @@ const workspaces = Files.join(process.cwd(), 'testables', 'workspaces') class NullUpdateScript extends UpdateScript { constructor() { - super(NullScriptName, UpdaterType.Root) + super(NullScriptName) } } class NullUpdateScripts extends UpdateScript { constructor() { - super(NullScriptsName, UpdaterType.Projects) + super(NullScriptsName) } } @@ -31,9 +31,4 @@ describe('when extending update scripts', () => { Registry.get(NullScriptName).exec(testables).then(() => done()) }) - it('should execute in workspaces project', (done) => { - const script = Registry.get(NullScriptsName) - Project.load(workspaces).then(project => script.workspace(project).then(() => done())) - }) - }) diff --git a/src/Core/UpdateScript.ts b/src/Core/UpdateScript.ts index 8525361..a2d1e71 100644 --- a/src/Core/UpdateScript.ts +++ b/src/Core/UpdateScript.ts @@ -1,16 +1,14 @@ import * as cp from 'child_process' import * as path from 'path' -import { Files, Log, Logger, Project, Updater, UpdaterType } from './index' +import { Files, Log, Logger, Project, Updater } from './index' export abstract class UpdateScript implements Updater { protected readonly log: Log private readonly _name: string - private readonly _type: UpdaterType - constructor(name: string, type: UpdaterType) { + constructor(name: string) { this._name = name - this._type = type this.log = Logger(name) } @@ -23,15 +21,13 @@ export abstract class UpdateScript implements Updater { return ['test', 'testing'].some(value => value === env) } - public get type(): UpdaterType { - return this._type + public exec(project: Project): Promise { + return project.children && project.children.length + ? Promise.all(project.children.map(child => this.workspace(child))).then(() => project) + : Promise.resolve(project) } - public exec(rootpath: string): Promise { - return Promise.resolve(Project.InvalidProject) - } - - public workspace(project: Project): Promise { + protected workspace(project: Project): Promise { return Promise.resolve(Project.InvalidProject) } From 80899a4106ea69f7e05049928131ab8e5f9161b3 Mon Sep 17 00:00:00 2001 From: Mike Pham Date: Sun, 26 Nov 2017 21:53:28 -0500 Subject: [PATCH 2/2] fix: organizing logging output --- src/Chest.ts | 6 +++--- src/Core/Actions/Packages.ts | 39 +++++++++++++++++++---------------- src/Core/Actions/Yarn.ts | 3 ++- src/Core/Files.ts | 2 +- src/Core/Logger.ts | 4 ++-- src/Core/UpdateScript.spec.ts | 34 ------------------------------ src/Core/UpdateScript.ts | 18 +++++++++++----- tsconfig.json | 4 ++++ 8 files changed, 46 insertions(+), 64 deletions(-) delete mode 100644 src/Core/UpdateScript.spec.ts diff --git a/src/Chest.ts b/src/Chest.ts index ee165b3..a7b80de 100644 --- a/src/Chest.ts +++ b/src/Chest.ts @@ -4,7 +4,7 @@ export class Chest { private static readonly log: Log = Logger('chest') public static run(cwd: string, ...args: string[]): Promise { - Chest.log.debug('run', cwd, args) + Chest.log.info('run', cwd, ...args) const updaters = Registry.all() return Project.load(cwd) @@ -12,8 +12,8 @@ export class Chest { Object.keys(updaters) .filter(scriptname => args.some(arg => arg.toLowerCase() === scriptname.toLowerCase())) .map(scriptname => updaters[scriptname]) - .map(script => script.exec(project)) + .map(script => script.exec(project).then(proj => Chest.log.task('done', '[project]', proj.name))) )) - .then(() => Chest.log.done('done', cwd)) + .then(() => Chest.log.done('done', cwd, args)) } } diff --git a/src/Core/Actions/Packages.ts b/src/Core/Actions/Packages.ts index 9af6e89..1a26559 100644 --- a/src/Core/Actions/Packages.ts +++ b/src/Core/Actions/Packages.ts @@ -13,24 +13,27 @@ class Script extends UpdateScript { super(Script.Name) } - protected async workspace(project: Project): Promise { - if (project.owner) { - const source = await project.owner.npm - const target = await project.npm - - target.author = source.author - target.bugs = source.bugs - target.description = source.description - target.homepage = source.homepage - target.license = source.license - target.repository = source.repository - - const filename = path.join(project.path, 'package.json') - await Files.save(filename, target) - this.log.task('workspace', filename) - } - - return project + protected workspace(project: Project): Promise { + return super.workspace(project) + .then(async () => { + if (project.owner) { + const source = await project.owner.npm + const target = await project.npm + + target.author = source.author + target.bugs = source.bugs + target.description = source.description + target.homepage = source.homepage + target.license = source.license + target.repository = source.repository + + const filename = path.join(project.path, 'package.json') + await Files.save(filename, target) + this.log.task('workspace', filename) + } + + return project + }) } } diff --git a/src/Core/Actions/Yarn.ts b/src/Core/Actions/Yarn.ts index e0d596d..f365515 100644 --- a/src/Core/Actions/Yarn.ts +++ b/src/Core/Actions/Yarn.ts @@ -15,7 +15,8 @@ export class Yarn extends UpdateScript { } protected workspace(project: Project): Promise { - return this.run(project, 'yarn').then(() => project) + return super.exec(project) + .then(project => this.run(project, 'yarn').then(() => project)) } } diff --git a/src/Core/Files.ts b/src/Core/Files.ts index ac3d458..cb5a55b 100644 --- a/src/Core/Files.ts +++ b/src/Core/Files.ts @@ -10,7 +10,7 @@ export interface Stat { } class InternalFiles { - private readonly log: Log = Logger('files') + private readonly log: Log = Logger('chest:files') public basename(filepath: string): string { return path.basename(filepath) diff --git a/src/Core/Logger.ts b/src/Core/Logger.ts index 4a39c36..9b69167 100644 --- a/src/Core/Logger.ts +++ b/src/Core/Logger.ts @@ -14,14 +14,14 @@ export function Logger(name: string, category?: string): Log { const bold = (name: string) => chalk.default.bold(`[${name}${cat}]`) const log = (...args: any[]) => { - if (process.env.DEBUG) { + if (process.env.DEBUG && process.env.NODE_ENV === 'development') { console.log(...args) } } return { debug: (...args: any[]): void => { - log(chalk.default.yellow.inverse(bold(name), ...args)) + log(chalk.default.yellow(bold(name), ...args)) }, error: (...args: any[]): void => { log(chalk.default.red.inverse(bold(name), ...args)) diff --git a/src/Core/UpdateScript.spec.ts b/src/Core/UpdateScript.spec.ts deleted file mode 100644 index d3ddec2..0000000 --- a/src/Core/UpdateScript.spec.ts +++ /dev/null @@ -1,34 +0,0 @@ -import 'mocha' - -import { expect } from 'chai' - -import { Files, Project, Registry, UpdateScript } from './index' - -const NullScriptName = 'null-update' -const NullScriptsName = 'null-updates' - -const testables = Files.join(process.cwd(), 'testables', 'single') -const workspaces = Files.join(process.cwd(), 'testables', 'workspaces') - -class NullUpdateScript extends UpdateScript { - constructor() { - super(NullScriptName) - } -} - -class NullUpdateScripts extends UpdateScript { - constructor() { - super(NullScriptsName) - } -} - -Registry.add(NullScriptName, new NullUpdateScript()) -Registry.add(NullScriptsName, new NullUpdateScripts()) - -describe('when extending update scripts', () => { - - it('should execute in root project', (done) => { - Registry.get(NullScriptName).exec(testables).then(() => done()) - }) - -}) diff --git a/src/Core/UpdateScript.ts b/src/Core/UpdateScript.ts index a2d1e71..d39d18d 100644 --- a/src/Core/UpdateScript.ts +++ b/src/Core/UpdateScript.ts @@ -9,7 +9,7 @@ export abstract class UpdateScript implements Updater { constructor(name: string) { this._name = name - this.log = Logger(name) + this.log = Logger(`chest:${name}`) } public get name(): string { @@ -22,13 +22,21 @@ export abstract class UpdateScript implements Updater { } public exec(project: Project): Promise { - return project.children && project.children.length - ? Promise.all(project.children.map(child => this.workspace(child))).then(() => project) - : Promise.resolve(project) + return ( + project.children && project.children.length + ? Promise.all( + project.children.map(child => this.workspace(child).then(proj => this.log.task(proj.name))) + ).then(() => project) + : Promise.resolve(project) + ) + .then(() => this.log.task(this.name, project.name)) + .then(() => project) } protected workspace(project: Project): Promise { - return Promise.resolve(Project.InvalidProject) + return Promise.resolve(project) + .then(() => this.log.task(this.name, project.name)) + .then(() => project) } protected npm(basepath: string): Promise { diff --git a/tsconfig.json b/tsconfig.json index 85153df..e3190ee 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -24,6 +24,10 @@ "strictNullChecks": true, "target": "es2015", "types": [ + "@types/chai", + "@types/chai-as-promised", + "@types/mocha", + "@types/node", "chalk", "tslint", "typescript"