From 952cfc4bd152fd029f9367b59afdcdf6bd798ec6 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Mar 2015 13:36:31 +0000 Subject: [PATCH 1/4] Speed up TypeScript compilation when running test suite Speed up the test suite by updating to the current version of the TypeScript compiler which is ~5x faster than the 1.0 version that was being used previously and caching compiled TypeScript files. * Update type definitions and flatten.ts for compatibility with TS 1.4 * Remove ts-compiler dependency and use the now-public TypeScript compiler API directly. Fixes #399 --- .gitignore | 1 + Gruntfile.js | 13 +----- __tests__/flatten.ts | 6 ++- package.json | 4 +- resources/jestPreprocessor.js | 77 ++++++++++++++++++++++++--------- type-definitions/Immutable.d.ts | 4 ++ 6 files changed, 68 insertions(+), 37 deletions(-) diff --git a/.gitignore b/.gitignore index 5de07b1848..095813c5e3 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ npm-debug.log *~ *.swp TODO +build diff --git a/Gruntfile.js b/Gruntfile.js index f703dfa2e6..28bb759046 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -203,17 +203,6 @@ module.exports = function(grunt) { }); }); - grunt.registerTask('init_ts_compiler', function () { - // LOL. This is because requiring ts-compiler for the first time actually - // calls fs.writeFileSync(), which when called from within a parallel - // test-runner, freaks out the file system and fails. This is pretty - // poor-form from TypeScript. The solution is to first require ts-compiler - // from this main process, before our test runner can access it, so - // fs.writeFileSync() has an opportunity to succeed. - var ts = require('ts-compiler'); - }); - - grunt.loadNpmTasks('grunt-contrib-jshint'); grunt.loadNpmTasks('grunt-contrib-copy'); grunt.loadNpmTasks('grunt-contrib-clean'); @@ -222,6 +211,6 @@ module.exports = function(grunt) { grunt.registerTask('lint', 'Lint all source javascript', ['jshint']); grunt.registerTask('build', 'Build distributed javascript', ['clean', 'bundle', 'copy']); - grunt.registerTask('test', 'Test built javascript', ['init_ts_compiler', 'jest']); + grunt.registerTask('test', 'Test built javascript', ['jest']); grunt.registerTask('default', 'Lint, build and test.', ['lint', 'build', 'stats', 'test']); } diff --git a/__tests__/flatten.ts b/__tests__/flatten.ts index a2a68a9cbc..1583939d35 100644 --- a/__tests__/flatten.ts +++ b/__tests__/flatten.ts @@ -8,6 +8,8 @@ import I = require('immutable'); import jasmineCheck = require('jasmine-check'); jasmineCheck.install(); +type SeqType = number | number[] | I.Iterable; + describe('flatten', () => { it('flattens sequences one level deep', () => { @@ -29,13 +31,13 @@ describe('flatten', () => { }) it('flattens only Sequences (not sequenceables)', () => { - var nested = I.Seq.of(I.Range(1,3),[3,4],I.List.of(5,6,7),8); + var nested = I.Seq.of(I.Range(1,3),[3,4],I.List.of(5,6,7),8); var flat = nested.flatten(); expect(flat.toJS()).toEqual([1,2,[3,4],5,6,7,8]); }) it('can be reversed', () => { - var nested = I.Seq.of(I.Range(1,3),[3,4],I.List.of(5,6,7),8); + var nested = I.Seq.of(I.Range(1,3),[3,4],I.List.of(5,6,7),8); var flat = nested.flatten(); var reversed = flat.reverse(); expect(reversed.toJS()).toEqual([8,7,6,5,[3,4],2,1]); diff --git a/package.json b/package.json index 5e49c3b4bc..15698ade14 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "magic-string": "^0.2.6", "microtime": "^1.2.0", "react-tools": "^0.11.1", - "ts-compiler": "^2.0.0", + "typescript": "^1.4.1", "uglify-js": "^2.4.15" }, "engines": { @@ -71,4 +71,4 @@ "iteration" ], "license": "BSD" -} \ No newline at end of file +} diff --git a/resources/jestPreprocessor.js b/resources/jestPreprocessor.js index 18a34f4f3a..e9213a7d2b 100644 --- a/resources/jestPreprocessor.js +++ b/resources/jestPreprocessor.js @@ -1,31 +1,66 @@ // preprocessor.js +var fs = require('fs'); var path = require('path'); -var ts = require('ts-compiler'); +var typescript = require('typescript'); var react = require('react-tools'); +var CACHE_DIR = path.join(path.resolve(__dirname + '/../build')); + +function isFileNewer(a, b) { + try { + return fs.statSync(a).mtime > fs.statSync(b).mtime; + } catch (ex) { + return false; + } +} + +function compileTypeScript(filePath) { + var options = { + outDir: CACHE_DIR, + noEmitOnError: true, + target: typescript.ScriptTarget.ES5, + module: typescript.ModuleKind.CommonJS + }; + + // re-use cached source if possible + var outputPath = path.join(options.outDir, path.basename(filePath, '.ts')) + '.js'; + if (isFileNewer(outputPath, filePath)) { + return fs.readFileSync(outputPath).toString(); + } + + if (fs.existsSync(outputPath)) { + fs.unlinkSync(outputPath); + } + + var host = typescript.createCompilerHost(options); + var program = typescript.createProgram([filePath], options, host); + var checker = typescript.createTypeChecker(program, true /* produceDiagnostics */); + var result = checker.emitFiles(); + + var allErrors = program.getDiagnostics().concat(checker.getDiagnostics()) + .concat(result.diagnostics); + allErrors.forEach(function(diagnostic) { + var lineChar = diagnostic.file.getLineAndCharacterFromPosition(diagnostic.start); + console.error('%s %d:%d %s', diagnostic.file.filename, lineChar.line, lineChar.character, diagnostic.messageText); + }); + + if (result.emitResultStatus !== typescript.EmitReturnStatus.Succeeded) { + throw new Error('Compiling ' + filePath + ' failed'); + } + + return fs.readFileSync(outputPath).toString(); +} + module.exports = { process: function(src, filePath) { if (filePath.match(/\.ts$/) && !filePath.match(/\.d\.ts$/)) { - ts.compile([filePath], { - skipWrite: true, - module: 'commonjs' - }, function(err, results) { - if (err) { - throw err; - } - results.forEach(function(file) { - // This is gross, but jest doesn't provide an asynchronous way to - // process a module, and ts currently runs syncronously. - src = file.text; - }); - }); - return src; - } - if (filePath.match(/\.js$/)) { - return react.transform(src, {harmony: true}).replace( - /require\('immutable/g, - "require('" + path.relative(path.dirname(filePath), process.cwd()) - ); + return compileTypeScript(filePath); + } else if (filePath.match(/\.js$/)) { + var result = react.transform(src, {harmony: true}).replace( + /require\('immutable/g, + "require('" + path.relative(path.dirname(filePath), process.cwd()) + ); + return result; } return src; } diff --git a/type-definitions/Immutable.d.ts b/type-definitions/Immutable.d.ts index 1ce4b7b1cf..0a1ecef4eb 100644 --- a/type-definitions/Immutable.d.ts +++ b/type-definitions/Immutable.d.ts @@ -1067,6 +1067,10 @@ declare module 'immutable' { new (): Map; new (values: {[key: string]: any}): Map; new (values: Iterable): Map; // deprecated + + (): Map; + (values: {[key: string]: any}): Map; + (values: Iterable): Map; // deprecated } } From 5d17feb7e67312b02088fa588cb22e6bbf5f6435 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Mar 2015 19:25:27 +0000 Subject: [PATCH 2/4] Fix incorrect expectation in slice test The test comment indicated that one trailing hole was expected but the argument to toEqual() had two. This used to pass previously due to an emission bug in the TypeScript 1.0 compiler which is fixed in TS 1.4 --- __tests__/slice.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/slice.ts b/__tests__/slice.ts index eb7ea4b0eb..6dc2a363b0 100644 --- a/__tests__/slice.ts +++ b/__tests__/slice.ts @@ -31,7 +31,7 @@ describe('slice', () => { it('slices a sparse indexed sequence', () => { expect(Seq([1,,2,,3,,4,,5,,6]).slice(1).toArray()).toEqual([,2,,3,,4,,5,,6]); expect(Seq([1,,2,,3,,4,,5,,6]).slice(2).toArray()).toEqual([2,,3,,4,,5,,6]); - expect(Seq([1,,2,,3,,4,,5,,6]).slice(3, -3).toArray()).toEqual([,3,,4,,,]); // one trailing hole. + expect(Seq([1,,2,,3,,4,,5,,6]).slice(3, -3).toArray()).toEqual([,3,,4,,]); // one trailing hole. }) it('can maintain indices for an keyed indexed sequence', () => { From 67ae47dec63bd08b2936fdb3c48159a1f5fedf9a Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Mar 2015 19:30:24 +0000 Subject: [PATCH 3/4] Remove reference to ts-compiler module which no longer exists --- resources/node_test.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/resources/node_test.sh b/resources/node_test.sh index ba86d70386..f1d8b56081 100755 --- a/resources/node_test.sh +++ b/resources/node_test.sh @@ -1,8 +1,5 @@ #!/bin/bash -# Many lols. See gruntfile for full explanation. -node -e "require('ts-compiler')" - # Run all tests using jest if [[ $TRAVIS ]] then jest -i # Travis tests are run inline From c42586fa0220b4eba4400a2225f745a7cf9ab950 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Mar 2015 19:36:05 +0000 Subject: [PATCH 4/4] Update type definitions in dist/ from the type-definitions/ folder --- dist/immutable.d.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dist/immutable.d.ts b/dist/immutable.d.ts index 1ce4b7b1cf..0a1ecef4eb 100644 --- a/dist/immutable.d.ts +++ b/dist/immutable.d.ts @@ -1067,6 +1067,10 @@ declare module 'immutable' { new (): Map; new (values: {[key: string]: any}): Map; new (values: Iterable): Map; // deprecated + + (): Map; + (values: {[key: string]: any}): Map; + (values: Iterable): Map; // deprecated } }