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

Implement fast incremental compiler #85

Merged
merged 46 commits into from
Feb 6, 2018
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
62a81de
implement fast incremental compiler
dwickern Nov 24, 2017
63d5e1e
use serve-ts command instead of preprocessor
dwickern Nov 25, 2017
e43d55b
Update lockfile and ember-try config.
chriskrycho Nov 25, 2017
b8248d9
fix first-time directory creation
dwickern Nov 25, 2017
0362674
add flag when running with serve-ts
dwickern Nov 25, 2017
1db94d5
Merge in `1.0.6` (Ember upgrade) changes.
chriskrycho Dec 17, 2017
0955fc3
Exclude blueprints from build.
chriskrycho Dec 17, 2017
18774d7
Enable node type-checking with @ts-check.
chriskrycho Dec 18, 2017
39a53d3
Fix lint and type errors in typescript-preprocessor.
chriskrycho Dec 18, 2017
3da13f9
Cleanup of `.e-c-ts` dir when done with `ts-serve`.
chriskrycho Dec 18, 2017
64484df
Implement cleanup of `.e-c-ts` dir.
chriskrycho Dec 18, 2017
2a2952b
Clean up tsc process on ^C.
chriskrycho Dec 18, 2017
5acf76c
serve-ts: resolve imports relative to the root project
dwickern Dec 18, 2017
069f3e5
serve-ts: print tsc output
dwickern Dec 18, 2017
87f5054
Add `.e.c-ts` to `.gitignore` on install.
chriskrycho Dec 22, 2017
9f7da6b
Use rimraf to clean up `.e-c-ts`.
chriskrycho Dec 23, 2017
9c64109
Clean up error message on setup failure.
chriskrycho Dec 23, 2017
b03e1c8
Some metadata and a ts-ignore for the `serve-ts` command.
chriskrycho Dec 23, 2017
8339612
Fix an import (`require` not `import`).
chriskrycho Dec 23, 2017
53012fc
Add VS Code exclude (roughly right, I think?).
chriskrycho Dec 23, 2017
f4316dd
“Prettier” error output, better broccoli debugging of trees.
chriskrycho Jan 16, 2018
2feb95a
Try using cross-env for node paths.
chriskrycho Jan 16, 2018
1327866
Drop `async`/`await`.
chriskrycho Jan 16, 2018
d5147d2
Revert "Try using cross-env for node paths."
chriskrycho Jan 16, 2018
59ab4fd
Handle `types` dir by convention.
chriskrycho Jan 22, 2018
8c42b49
Include *all* directories included via `include` as well.
chriskrycho Jan 22, 2018
624b70c
Allow use of environment variable for npm link scenario.
chriskrycho Jan 22, 2018
77f13e5
Solve 'The Mystery of the Missing app.js 🕵️‍♂️'
dfreeman Jan 24, 2018
3f722d9
Merge pull request #89 from dfreeman/fast-incremental-compiler
dwickern Jan 24, 2018
da18bba
Fix cleanup
dfreeman Jan 24, 2018
54edcdc
Move incremental output to OS temp
dfreeman Jan 24, 2018
8fad31c
Minimize rebuild thrashing
dfreeman Jan 24, 2018
0e4b8b5
Fix app URL in serve output
dfreeman Jan 24, 2018
fe5035e
Meltdown aftermath: sudo required in Travis.
chriskrycho Jan 24, 2018
a666892
Merge pull request #90 from dfreeman/fast-incremental-compiler
chriskrycho Jan 24, 2018
0b4b4fe
Better approach for Meltdown mitigation Chrome handling.
chriskrycho Jan 24, 2018
59e2c77
Do not override project-level sourcemap setting.
chriskrycho Jan 24, 2018
ea8a494
Avoid double-including files
dfreeman Feb 4, 2018
b70d390
Spike out a Broccoli-integrated incremental compiler so regular ember…
dfreeman Feb 5, 2018
bdde2e9
Fix addons' `app` trees and cleanup dead code
dfreeman Feb 5, 2018
0cb4e0b
Merge pull request #92 from dfreeman/incremental-with-broccoli
chriskrycho Feb 5, 2018
5476b65
Fix the dummy app
dfreeman Feb 5, 2018
71ac9d0
Merge branch 'master' into fast-incremental-compiler
chriskrycho Feb 5, 2018
365214d
Fix rebuilds triggered by non-TS files.
dfreeman Feb 5, 2018
6f6c5d3
At least give tsc a *chance* to catch up
dfreeman Feb 5, 2018
e96c0ca
`writeErrorLine` -> `writeError`. Also: Prettier.
chriskrycho Feb 5, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ language: node_js
node_js:
- "6"

sudo: false
sudo: required
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue described here and here on Travis' end.

dist: trusty

addons:
Expand Down
3 changes: 2 additions & 1 deletion blueprints/ember-cli-typescript/files/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ inRepoAddons.forEach(function(path) { %>,
"tmp",
"dist",
"node_modules",
"bower_components"
"bower_components",
"blueprints"
]
}
17 changes: 17 additions & 0 deletions blueprints/ember-cli-typescript/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-env node */

const { existsSync } = require('fs');
const path = require('path');

module.exports = {
Expand Down Expand Up @@ -37,6 +38,22 @@ module.exports = {
},

afterInstall() {
if (existsSync('.gitignore')) {
this.insertIntoFile('.gitignore', '\n# Ember CLI TypeScript\n.e-c-ts');
}

if (existsSync('.vscode/settings.json')) {
// NOTE: this may or may not actually work -- it assumes that
// `"tmp:": true` is in fact set.
this.insertIntoFile('.vscode/settings.json', '",\n.e-c-ts": true', {
after: '"tmp": true',
});
}

// TODO: same for Atom
// TODO: same for Sublime
// TODO: same for IntelliJ
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this whole afterInstall hook be avoidable if the output directory lived in either system temp (os.tmpdir()) or piggybacked off the local broccoli temporary directory (which is presumably already set up to be ignored by the user's editor, git, npm, etc)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think so. And if we can piggy-back off of os.tmpdir(), that seems reasonable enough to me, though making sure that gets pulled into the Broccoli tree seems… iffy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We rely on watchman (or other watcher, but I've only tested with watchman) to pick up the compiled js.

Watchman is set up to monitor the root directory of the app, minus the excluded directories in the default .watchmanconfig:

{
  "ignore_dirs": ["tmp", "dist"]
}

So we can't put the files in tmp or some other system directory

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible we could add another watched directory somehow but I haven't explored the APIs for it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is it would end up adding that directory as a watchman root, but I'm pretty unfamiliar with the details there, so grain of salt. Best bet is probably just to try it and see what happens ¯\_(ツ)_/¯


return this.addPackagesToProject([
{ name: 'typescript', target: 'latest' },
{ name: '@types/ember', target: 'latest' },
Expand Down
109 changes: 106 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,113 @@
// @ts-check
/* eslint-env node */

const fs = require('fs');
const path = require('path');
const SilentError = require('silent-error');
const TsPreprocessor = require('./lib/typescript-preprocessor');
const buildServeCommand = require('./lib/serve-ts');
const funnel = require('broccoli-funnel');
const mergeTrees = require('broccoli-merge-trees');
const mkdirp = require('mkdirp');

module.exports = {
name: 'ember-cli-typescript',

_isRunningServeTS() {
return this.project._isRunningServeTS;
},

_inRepoAddons() {
const pkg = this.project.pkg;
if (!pkg || !pkg['ember-addon'] || !pkg['ember-addon'].paths) {
return [];
}

return pkg['ember-addon'].paths;
},

includedCommands() {
return {
'serve-ts': buildServeCommand(this.project),
};
},

// Stolen from ember-cli-mirage.
included() {
let app;

// If the addon has the _findHost() method (in ember-cli >= 2.7.0), we'll just
// use that.
if (typeof this._findHost === 'function') {
app = this._findHost();
} else {
// Otherwise, we'll use this implementation borrowed from the _findHost()
// method in ember-cli.
let current = this;
do {
app = current.app || app;
} while (current.parent.parent && (current = current.parent));
}

this.app = app;

this._super.included.apply(this, arguments);
},

treeForApp(tree) {
const { include } = JSON.parse(
fs.readFileSync(path.resolve(this.app.project.root, 'tsconfig.json'), { encoding: 'utf8' })
);

const includes = ['types']
.concat(include ? include : [])
.reduce((unique, entry) => (unique.indexOf(entry) === -1 ? unique.concat(entry) : unique), [])
.map(p => path.resolve(this.app.project.root, p))
.filter(fs.existsSync);

const additionalTrees = includes.map(p => funnel(p, { destDir: p }));

if (!this._isRunningServeTS()) {
return mergeTrees([tree, ...additionalTrees]);
}

const roots = ['.', ...includes, ...this._inRepoAddons()].map(root => path.join(root, 'app/'));

// funnel will fail if the directory doesn't exist
roots.forEach(root => {
mkdirp.sync(path.join('.e-c-ts', root));
});

const ts = funnel('.e-c-ts', {
exclude: ['tests'],
getDestinationPath(relativePath) {
const prefix = roots.find(root => relativePath.startsWith(root));
if (prefix) {
// strip any app/ or lib/in-repo-addon/app/ prefix
return relativePath.substr(prefix.length);
}

return relativePath;
},
});

return mergeTrees([tree, ts]);
},

treeForTestSupport(tree) {
if (!this._isRunningServeTS()) {
return tree;
}

const tests = path.join('.e-c-ts', 'tests');

// funnel will fail if the directory doesn't exist
mkdirp.sync(tests);

const ts = funnel(tests);
return tree ? mergeTrees([tree, ts]) : ts;
},

setupPreprocessorRegistry(type, registry) {
if (!fs.existsSync(path.join(this.project.root, 'tsconfig.json'))) {
// Do nothing; we just won't have the plugin available. This means that if you
Expand All @@ -23,6 +123,11 @@ module.exports = {
return;
}

if (type === 'self' || this._isRunningServeTS()) {
// TODO: still need to compile TS addons
return;
}

try {
registry.add(
'js',
Expand All @@ -32,9 +137,7 @@ module.exports = {
);
} catch (ex) {
throw new SilentError(
`Failed to instantiate TypeScript preprocessor, probably due to an invalid tsconfig.json. Please fix or run \`ember generate ember-cli-typescript\`.\n${
ex
}`
`Failed to instantiate TypeScript preprocessor, probably due to an invalid tsconfig.json. Please fix or run \`ember generate ember-cli-typescript\`.\n${ex}`
);
}
},
Expand Down
104 changes: 104 additions & 0 deletions lib/serve-ts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// @ts-check
/* eslint-env node */

const child_process = require('child_process');
const fs = require('fs');

const rimraf = require('rimraf');
const OUT_DIR = '.e-c-ts';

module.exports = (project) => {
const Serve = project.require('ember-cli/lib/commands/serve');
const Builder = project.require('ember-cli/lib/models/builder');
const Watcher = project.require('ember-cli/lib/models/watcher');

/**
* Exclude .ts files from being watched
*/
function filterTS(name) {
if (name.startsWith('.')) {
// these files are filtered by default
return false;
}

// typescript sources are watched by `tsc --watch` instead
return !name.endsWith('.ts');
}

class WatcherNonTS extends Watcher {
buildOptions() {
let options = super.buildOptions();
options.filter = filterTS;
return options;
}
}

return Serve.extend({
name: 'serve-ts',
aliases: ['st'],
works: 'insideProject',
description:
'Serve the app/addon with the TypeScript compiler in incremental mode. (Much faster!)',

run(options) {
this.project._isRunningServeTS = true;

const builder = new Builder({
ui: this.ui,
outputPath: options.outputPath,
project: this.project,
environment: options.environment,
});

// We're ignoring this because TS doesn't have types for `Watcher`; this is
// fine, though.
// @ts-ignore
const watcher = new WatcherNonTS({
ui: this.ui,
builder,
analytics: this.analytics,
options,
serving: true,
});

options._builder = builder;
options._watcher = watcher;

// TODO: typescript might be installed globally?
// argument sequence here is meaningful; don't apply prettier.
// prettier-ignore
this.tsc = child_process.fork(
'node_modules/typescript/bin/tsc',
[
'--watch',
'--outDir', OUT_DIR,
'--allowJs', 'false',
'--noEmit', 'false',
'--sourceMap', 'false', // TODO: enable if sourcemaps=true
],
{
silent: true,
execArgv: [],
}
);
this.tsc.stdout.on('data', data => {
this.ui.write(data);
});
this.tsc.stderr.on('data', data => {
this.ui.writeError(data);
});

return Serve.prototype.run.call(this, options);
},

onInterrupt() {
if (this.tsc) {
this.tsc.kill();
}

if (fs.existsSync(OUT_DIR)) {
rimraf.sync(OUT_DIR);
}
},
});
};
36 changes: 22 additions & 14 deletions lib/typescript-preprocessor.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// @ts-check
/* eslint-env node */

const fs = require('fs');
const path = require('path');

const funnel = require('broccoli-funnel');
const mergeTrees = require('broccoli-merge-trees');
const TypeScriptPlugin = require('broccoli-typescript-compiler').TypeScriptPlugin;
const { typescript } = require('broccoli-typescript-compiler');

const BroccoliDebug = require('broccoli-debug');

Expand All @@ -13,50 +15,56 @@ let tag = 0;
class TypeScriptPreprocessor {
constructor(options) {
this.name = 'ember-cli-typescript';
this._tag = tag++;
this._tag = tag;
this.ext = 'ts';
this.ui = options.ui;

// Update the config for how Broccoli handles the file system: no need for
// includes, always emit, and let Broccoli manage any outDir.
this.config = JSON.parse(fs.readFileSync(path.join(process.cwd(), 'tsconfig.json')));
const config = fs.readFileSync(path.join(process.cwd(), 'tsconfig.json'), { encoding: 'utf8' });
this.config = JSON.parse(config);
this.config.compilerOptions.noEmit = false;
this.config.compilerOptions.allowJs = false;
delete this.config.compilerOptions.outDir;
delete this.config.include;
}

toTree(inputNode, inputPath, outputPath) {
toTree(inputNode /*, inputPath, outputPath*/) {
// increment every time toTree is called so we have some idea what's going on here.
this._tag = tag++;

const debugTree = BroccoliDebug.buildDebugCallback('ember-cli-typescript');

const js = funnel(inputNode, {
exclude: ['**/*.ts'],
annotation: 'JS files',
});

const debugTree = BroccoliDebug.buildDebugCallback('ember-cli-typescript');

const uncompiledTs = debugTree(
funnel(inputNode, {
include: ['**/*.ts'],
annotation: 'uncompiled TS files',
}),
`${this._tag}`
`${this._tag}:uncompiled-ts`
);

const tsc = new TypeScriptPlugin(uncompiledTs, {
const tsc = typescript(uncompiledTs, {
throwOnError: this.config.compilerOptions.noEmitOnError,
annotation: 'Compiled TS files',
include: ['**/*'],
tsconfig: this.config,
});
tsc.setDiagnosticWriter(this.ui.writeWarnLine.bind(this.ui));

const ts = debugTree(tsc, `${this._tag}`);
const ts = debugTree(tsc, `${this._tag}:compiled-ts`);

// Put everything together.
return mergeTrees([js, ts], {
overwrite: true,
annotation: 'merged JS & compiled TS',
});
return debugTree(
mergeTrees([js, ts], {
overwrite: true,
annotation: 'merged JS & compiled TS',
}),
`${this._tag}:final`
);
}
}

Expand Down
Loading