-
Notifications
You must be signed in to change notification settings - Fork 14
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(cli): convert to shallow, precompiled builds
building each addon every time for every app introduced a significant performance problem - cold boots took upwards of 10s with a minimal app. Furthermore, debugging was much more difficult thanks to the mirrored compiled addon structure in dist/node_modules. This changes denali to only build the app/addon you run the command inside, but adds the ability to mark a linked addon dependency as "under "development", which causes denali to also watch and rebuild that linked dependency's source. This introduces additional burden on addon developers (they must publish their `dist/` folder, **not** the project root). But the benefits for all users outweigh the cost to addon developers, and those caveats can be clearly documented.
- Loading branch information
1 parent
5d2def6
commit 9e5e20d
Showing
9 changed files
with
154 additions
and
165 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,63 +5,53 @@ import Funnel from 'broccoli-funnel'; | |
| import MergeTree from 'broccoli-merge-trees'; | ||
| import PackageTree from './package-tree'; | ||
| import discoverAddons from '../utils/discover-addons'; | ||
| import tryRequire from '../utils/try-require'; | ||
| import createDebug from 'debug'; | ||
|
|
||
| const debug = createDebug('denali:builder'); | ||
|
|
||
|
|
||
| export default class Builder { | ||
|
|
||
| static createFor(dir, project) { | ||
| debug(`creating builder for ${ dir }`); | ||
| // Use the local denali-build.js if present | ||
| let denaliBuildPath = path.join(dir, 'denali-build'); | ||
| if (fs.existsSync(`${ denaliBuildPath }.js`)) { | ||
| let LocalBuilder = require(denaliBuildPath); | ||
| LocalBuilder = LocalBuilder.default || LocalBuilder; | ||
| return new LocalBuilder(dir, project); | ||
| } | ||
| return new this(dir, project); | ||
| } | ||
|
|
||
| ignoreVulnerabilities = [ | ||
| [ '[email protected]' ] | ||
| ]; | ||
|
|
||
| constructor(dir, project, preseededAddons) { | ||
| // Use the local denali-build.js if present | ||
| let LocalBuilder = tryRequire(path.join(dir, 'denali-build')); | ||
| if (LocalBuilder && this.constructor === Builder) { | ||
| return new LocalBuilder(dir, project, preseededAddons); | ||
| } | ||
| packageFiles = [ | ||
| 'package.json', | ||
| 'README.md', | ||
| 'CHANGELOG.md', | ||
| 'LICENSE', | ||
| 'denali-build.js' | ||
| ]; | ||
|
|
||
| unbuiltDirs = [ | ||
| 'blueprints', | ||
| 'commands' | ||
| ]; | ||
|
|
||
| constructor(dir, project) { | ||
| this.dir = dir; | ||
| this.pkg = require(path.join(this.dir, 'package.json')); | ||
| this.project = project; | ||
| this.preseededAddons = preseededAddons; | ||
| this.dest = path.relative(project.dir, this.dir); | ||
| if (this.isAddon && this.isProjectRoot) { | ||
| this.dest = path.join('node_modules', this.pkg.name); | ||
| } else if (this.dest === path.join('test', 'dummy')) { | ||
| this.dest = '.'; | ||
| } | ||
| this.lint = this.isProjectRoot ? project.lint : false; | ||
| // Inherit the environment from the project, *except* when this builder is | ||
| // representing an addon dependency and the environment is test. Basically, | ||
| // when we run tests, we don't want addon dependencies building for test. | ||
| this.environment = !this.isProjectRoot && project.environment === 'test' | ||
| ? 'development' | ||
| : project.environment; | ||
|
|
||
| // Register with the project; this must happen before child builders are | ||
| // created to ensure that any child addons that share this dependency will | ||
| // use this builder, rather than attempting to create their own because none | ||
| // existed in the project builders map. | ||
| this.project.builders.set(dir, this); | ||
|
|
||
| // Find child addons | ||
| let addons = discoverAddons(this.dir, { preseededAddons: this.preseededAddons }); | ||
| // Build a list of child addons for the processParent hook later | ||
| this.childBuilders = addons.map((addonDir) => this.project.builderFor(addonDir)); | ||
| } | ||
|
|
||
| get isAddon() { | ||
| return this.pkg.keywords && this.pkg.keywords.includes('denali-addon'); | ||
| } | ||
|
|
||
| get isProjectRoot() { | ||
| return this.project.dir === this.dir; | ||
| this.isAddon = this.pkg.keywords && this.pkg.keywords.includes('denali-addon'); | ||
| this.addons = discoverAddons(this.dir); | ||
| } | ||
|
|
||
| sourceDirs() { | ||
| let dirs = [ 'app', 'config', 'lib' ]; | ||
| if (this.environment === 'test') { | ||
| if (this.project.environment === 'test') { | ||
| dirs.push('test'); | ||
| } | ||
| return dirs; | ||
|
|
@@ -87,9 +77,9 @@ export default class Builder { | |
| return false; | ||
| }).filter(Boolean); | ||
|
|
||
| // Copy the package.json file into our build output (this special tree is | ||
| // Copy top level files into our build output (this special tree is | ||
| // necessary because broccoli can't pick a file from the root dir). | ||
| sourceTrees.push(new PackageTree(this.pkg)); | ||
| sourceTrees.push(new PackageTree(this, { files: this.packageFiles })); | ||
|
|
||
| // Combine everything into our unified source tree, ready for building | ||
| return new MergeTree(sourceTrees, { overwrite: true }); | ||
|
|
@@ -98,51 +88,32 @@ export default class Builder { | |
| toTree() { | ||
| let tree = this._prepareSelf(); | ||
|
|
||
| // Find child addons | ||
| this.childBuilders = this.addons.map((addonDir) => Builder.createFor(addonDir, this.project)); | ||
|
|
||
| // Run processParent hooks | ||
| this.childBuilders.forEach((builder) => { | ||
| if (builder.processParent) { | ||
| tree = builder.processParent(tree, this.dir); | ||
| } | ||
| }); | ||
|
|
||
| // Run processEach hooks | ||
| this.project.builders.forEach((builder) => { | ||
| if (builder.processEach) { | ||
| tree = builder.processEach(tree, this.dir); | ||
| } | ||
| }); | ||
|
|
||
| // Run processApp hooks | ||
| if (this.isProjectRoot) { | ||
| this.project.builders.forEach((builder) => { | ||
| if (builder.processApp) { | ||
| tree = builder.processApp(tree, this.dir); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // Run processSelf hooks | ||
| if (this.processSelf) { | ||
| tree = this.processSelf(tree, this.dir); | ||
| } | ||
|
|
||
| // If this is an addon, the project root, and we are building | ||
| // for "test", we want to move the tests from the addon up to the dummy | ||
| // app so they are actually run, but move everything else into | ||
| // node_modules like normal. | ||
| if (this.isAddon && this.isProjectRoot && this.environment === 'test') { | ||
| let addonTests = new Funnel(tree, { | ||
| include: [ 'test/**/*' ], | ||
| exclude: [ 'test/dummy/**/*' ] | ||
| }); | ||
| let addonWithoutTests = new Funnel(tree, { | ||
| exclude: [ 'test/**/*' ], | ||
| destDir: this.dest | ||
| }); | ||
| return new MergeTree([ addonWithoutTests, addonTests ]); | ||
| let unbuiltTrees = []; | ||
| this.unbuiltDirs.forEach((dir) => { | ||
| if (fs.existsSync(path.join(this.dir, dir))) { | ||
| unbuiltTrees.push(new Funnel(path.join(this.dir, dir), { destDir: dir })); | ||
| } | ||
| }); | ||
| if (unbuiltTrees.length > 0) { | ||
| tree = new MergeTree(unbuiltTrees.concat(tree), { overwrite: true }); | ||
| } | ||
|
|
||
| return new Funnel(tree, { destDir: this.dest }); | ||
| return tree; | ||
| } | ||
|
|
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,41 @@ | ||
| import path from 'path'; | ||
| import fs from 'fs'; | ||
| import Plugin from 'broccoli-plugin'; | ||
| import cloneDeep from 'lodash/cloneDeep'; | ||
|
|
||
| export default class PackageTree extends Plugin { | ||
|
|
||
| constructor(pkg, options) { | ||
| constructor(builder, options) { | ||
| super([], options); | ||
| this.pkg = pkg; | ||
| this.builder = builder; | ||
| this.dir = builder.dir; | ||
| this.isAddon = builder.pkg.keywords && builder.pkg.keywords.includes('denali-addon'); | ||
| this.files = options.files; | ||
| } | ||
|
|
||
| build() { | ||
| fs.writeFileSync(path.join(this.outputPath, 'package.json'), JSON.stringify(this.pkg, null, 2)); | ||
| // Copy over any top level files specified | ||
| this.files.forEach((file) => { | ||
| let src = path.join(this.dir, file); | ||
| let dest = path.join(this.outputPath, file); | ||
| if (fs.existsSync(src)) { | ||
| fs.writeFileSync(dest, fs.readFileSync(src)); | ||
| } | ||
| }); | ||
|
|
||
| // Addons should publish their dist directories, not the root project directory. | ||
| // To enforce this, the addon blueprint ships with a prepublish script that | ||
| // fails immediately, telling the user to run `denali publish` instead (which | ||
| // tests the addon, builds it, then runs npm publish from the dist folder). | ||
| // However, Denali itself would get blocked by our prepublish blocker too, | ||
| // so when we build an addon, we remove that blocker. But if the user has | ||
| // changed the prepublish script, then we leave it alone. | ||
| let scripts = this.builder.pkg.scripts; | ||
| if (scripts && scripts.prepublish === 'echo "Use \'denali publish\' instead." && exit 1') { | ||
| let pkg = cloneDeep(this.builder.pkg); | ||
| delete pkg.scripts.prepublish; | ||
| fs.writeFileSync(path.join(this.outputPath, 'package.json'), JSON.stringify(pkg, null, 2)); | ||
| } | ||
| } | ||
|
|
||
| } |
Oops, something went wrong.