X Tutup
Skip to content

chore(build): Clean-up TypeScript build #7941

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.

Sign up for GitHub

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

Closed
chuckjaz wants to merge 1 commit into from
Closed

chore(build): Clean-up TypeScript build #7941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 21 additions & 42 deletions tools/broccoli/broccoli-typescript.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ class DiffingTSCompiler implements DiffingBroccoliPlugin {
private genInternalTypings: boolean = false;

static includeExtensions = ['.ts'];
static excludeExtensions = ['.d.ts'];

constructor(public inputPath: string, public cachePath: string, public options) {
if (options.rootFilePaths) {
Expand All @@ -83,11 +82,7 @@ class DiffingTSCompiler implements DiffingBroccoliPlugin {
this.genInternalTypings = false;
}

// TODO: the above turns rootDir set to './' into an empty string - looks like a tsc bug
// check back when we upgrade to 1.7.x
if (this.tsOpts.rootDir === '') {
this.tsOpts.rootDir = './';
}
this.tsOpts.rootDir = inputPath;
this.tsOpts.outDir = this.cachePath;

this.tsServiceHost = new CustomLanguageServiceHost(this.tsOpts, this.rootFilePaths,
Expand Down Expand Up @@ -331,54 +326,34 @@ class CustomLanguageServiceHost implements ts.LanguageServiceHost {
}


getScriptFileNames(): string[] { return this.fileNames; }
getScriptFileNames(): string[] {
return this.fileNames.map(f => path.join(this.treeInputPath, f));
Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar Apr 6, 2016

Choose a reason for hiding this comment

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

this this right? shouldn't we achieve this by setting rootDir to inputPath instead?

Copy link
Copy Markdown
Contributor Author

@chuckjaz chuckjaz Apr 6, 2016

Choose a reason for hiding this comment

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

They are both the same but I can change this to this.compilerOptions.rootDir if that is more clear.

Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar Apr 7, 2016

Choose a reason for hiding this comment

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

the rootDir is already set to inputPath. no?

Copy link
Copy Markdown
Contributor Author

@chuckjaz chuckjaz Apr 7, 2016

Choose a reason for hiding this comment

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

Yes. They are equivalent. I felt it was more clear with this.treeInputPath as it is much more clear that these files are from the broccoli input tree.

Copy link
Copy Markdown
Contributor Author

@chuckjaz chuckjaz Apr 7, 2016

Choose a reason for hiding this comment

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

I misunderstood what you were saying here. Sorry about that.

TypeScript requires all files to be under rootDir and doesn't resolve relative files returned via getScriptFileNames() to rootDir. It is expected that getScriptFileNames() returns resolved filenames which is what this does. Changing rootDir, at least for 1.7.5, is not sufficient.

}


getScriptVersion(fileName: string): string {
return this.fileRegistry[fileName] && this.fileRegistry[fileName].version.toString();
}


/**
* This method is called quite a bit to lookup 3 kinds of paths:
* 1/ files in the fileRegistry
* - these are the files in our project that we are watching for changes
* - in the future we could add caching for these files and invalidate the cache when
* the file is changed lazily during lookup
* 2/ .d.ts and library files not in the fileRegistry
* - these are not our files, they come from tsd or typescript itself
* - these files change only rarely but since we need them very rarely, it's not worth the
* cache invalidation hassle to cache them
* 3/ bogus paths that typescript compiler tries to lookup during import resolution
* - these paths are tricky to cache since files come and go and paths that was bogus in the
* past might not be bogus later
*
* In the initial experiments the impact of this caching was insignificant (single digit %) and
* not worth the potential issues with stale cache records.
*/
getScriptSnapshot(tsFilePath: string): ts.IScriptSnapshot {
let absoluteTsFilePath;

if (tsFilePath == this.defaultLibFilePath || path.isAbsolute(tsFilePath)) {
absoluteTsFilePath = tsFilePath;
} else if (this.compilerOptions.moduleResolution === ts.ModuleResolutionKind.NodeJs &&
tsFilePath.match(/^node_modules/)) {
absoluteTsFilePath = path.resolve(tsFilePath);
} else if (tsFilePath.match(/^rxjs/)) {
absoluteTsFilePath = path.resolve('node_modules', tsFilePath);
} else if (tsFilePath.match(/^node_modules/)) {
absoluteTsFilePath = path.resolve('node_modules/../', tsFilePath);
} else {
absoluteTsFilePath = path.join(this.treeInputPath, tsFilePath);
}
// TypeScript seems to request lots of bogus paths during import path lookup and resolution,
// so we we just return undefined when the path is not correct.

// Ensure it is in the input tree or a lib.d.ts file.
if (!startsWith(tsFilePath, this.treeInputPath) && !tsFilePath.match(/\/lib(\..*)*.d\.ts$/)) {
if (fs.existsSync(tsFilePath)) {
console.log('Rejecting', tsFilePath, '. File is not in the input tree.');
}
return undefined;
}

if (!fs.existsSync(absoluteTsFilePath)) {
// TypeScript seems to request lots of bogus paths during import path lookup and resolution,
// so we we just return undefined when the path is not correct.
// Ensure it exists
if (!fs.existsSync(tsFilePath)) {
return undefined;
}
return ts.ScriptSnapshot.fromString(fs.readFileSync(absoluteTsFilePath, FS_OPTS));

return ts.ScriptSnapshot.fromString(fs.readFileSync(tsFilePath, FS_OPTS));
}


Expand All @@ -402,6 +377,10 @@ function clone<T>(object: T): T {
return <T>result;
}

function startsWith(str: string, substring: string): boolean {
return str.substring(0, substring.length) === substring;
}

function endsWith(str: string, substring: string): boolean {
return str.indexOf(substring, str.length - substring.length) !== -1;
}
7 changes: 6 additions & 1 deletion tools/broccoli/trees/browser_tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,18 @@ module.exports = function makeBrowserTree(options, destinationPath) {
{include: ['**/**'], exclude: ['e2e_test/**'], destDir: '/benchpress/'});
}

let externalTypings =
new Funnel('node_modules', {include: ['rxjs/**/*.d.ts', 'zone.js/**/*.d.ts']});


var modulesTree = mergeTrees([
angular2Tree,
benchmarksTree,
benchmarksExternalTree,
payloadTestsTree,
playgroundTree,
benchpressTree
benchpressTree,
externalTypings,
]);

var es6PolyfillTypings =
Expand Down
29 changes: 22 additions & 7 deletions tools/broccoli/trees/node_tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,28 @@ module.exports = function makeNodeTree(projects, destinationPath) {
'angular2/src/upgrade/**',
'angular2/upgrade.ts',
'angular2/platform/testing/**',
'angular2/manual_typings/**',
'angular2/typings/**'
]
});

let ambientTypings = [
let externalTypings = [
'angular2/typings/hammerjs/hammerjs.d.ts',
'angular2/typings/node/node.d.ts',
'node_modules/zone.js/dist/zone.js.d.ts',
'angular2/manual_typings/globals.d.ts',
'angular2/typings/es6-collections/es6-collections.d.ts',
'angular2/typings/es6-promise/es6-promise.d.ts'
];

let externalTypingsTree = new Funnel('modules', {files: externalTypings});

let packageTypings =
new Funnel('node_modules', {include: ['rxjs/**/*.d.ts', 'zone.js/**/*.d.ts']});

let compileSrcContext = mergeTrees([srcTree, externalTypingsTree, packageTypings]);

// Compile the sources and generate the @internal .d.ts
let compiledSrcTreeWithInternals = compileTree(srcTree, true, ambientTypings);
let compiledSrcTreeWithInternals = compileTree(compileSrcContext, true, []);

var testTree = new Funnel('modules', {
include: [
Expand Down Expand Up @@ -71,6 +79,8 @@ module.exports = function makeNodeTree(projects, destinationPath) {
'angular2/test/platform/xhr_impl_spec.ts',
'angular2/test/platform/browser/**/*.ts',
'angular2/test/common/forms/**',
'angular2/manual_typings/**',
'angular2/typings/**',

// we call browser's bootstrap
'angular2/test/router/route_config/route_config_spec.ts',
Expand All @@ -92,12 +102,17 @@ module.exports = function makeNodeTree(projects, destinationPath) {
let srcPrivateDeclarations =
new Funnel(compiledSrcTreeWithInternals, {srcDir: INTERNAL_TYPINGS_PATH});

testTree = mergeTrees([testTree, srcPrivateDeclarations]);

let compiledTestTree = compileTree(testTree, false, ambientTypings.concat([
let testAmbients = [
'angular2/typings/jasmine/jasmine.d.ts',
'angular2/typings/angular-protractor/angular-protractor.d.ts',
]));
'angular2/typings/selenium-webdriver/selenium-webdriver.d.ts'
];
let testAmbientsTree = new Funnel('modules', {files: testAmbients});

testTree = mergeTrees(
[testTree, srcPrivateDeclarations, testAmbientsTree, externalTypingsTree, packageTypings]);

let compiledTestTree = compileTree(testTree, false, []);

// Merge the compiled sources and tests
let compiledSrcTree =
Expand Down
X Tutup