feature #101 Forked typescript type checking (davidmpaz)

This PR was squashed before being merged into the master branch (closes #101).

Discussion
----------

Forked typescript type checking

This PR fixes: #63 it should be possible to merge also with #99 directly.

Open points:

Regarding point .1 in #2

> If this plugin is added. we need to check dependency is added using loader-features.js. This module name is getting name from loaders, we are not adding a loader here but a plugin. Maybe some more general module is needed for checking features?

After implementing this I realize that as  soon as you require a module which doesn't exist node throws an error, we don't even go through error catch from encore. So possible solutions I see for this pending point are:

1. We don't refactor `loader-features.js` file and we still will get a loud error when trying to use some plugin not installed. From error is clear whats need to be done.
2. We refactor `loader-features.js` to be named more general and we include plugins dependencies also there as intended initially. But this way we need to check for missing packages earlier in `WebpackConfig.js` like I am doing now in this PR.

I like either solutions the same, I would like to get feedback on what do you guys think about it?

thanks in advance.

Commits
-------

7a7e399 Forked typescript type checking

Author: weaverryan

index.js

@@ -390,6 +390,23 @@ module.exports = {
         return this;
     },
 
+    /**
+     * Call this to enable forked type checking for TypeScript loader
+     * https://github.com/TypeStrong/ts-loader/blob/v2.3.0/README.md#faster-builds
+     *
+     * This is a build optimization API to reduce build times.
+     *
+     * @param {function} forkedTypeScriptTypesCheckOptionsCallback
+     * @return {exports}
+     */
+    enableForkedTypeScriptTypesChecking(forkedTypeScriptTypesCheckOptionsCallback = () => {}) {
+        webpackConfig.enableForkedTypeScriptTypesChecking(
+            forkedTypeScriptTypesCheckOptionsCallback
+        );
+
+        return this;
+    },
+
     /**
      * If enabled, the Vue.js loader is enabled.
      *

lib/WebpackConfig.js

@@ -57,6 +57,8 @@ class WebpackConfig {
         this.loaders = [];
         this.useTypeScriptLoader = false;
         this.tsConfigurationCallback = function() {};
+        this.useForkedTypeScriptTypeChecking = false;
+        this.forkedTypeScriptTypesCheckOptionsCallback = () => {};
     }
 
     getContext() {
@@ -245,6 +247,17 @@ class WebpackConfig {
         this.tsConfigurationCallback = callback;
     }
 
+    enableForkedTypeScriptTypesChecking(forkedTypeScriptTypesCheckOptionsCallback = () => {}) {
+
+        if (typeof forkedTypeScriptTypesCheckOptionsCallback !== 'function') {
+            throw new Error('Argument 1 to enableForkedTypeScriptTypesChecking() must be a callback function.');
+        }
+
+        this.useForkedTypeScriptTypeChecking = true;
+        this.forkedTypeScriptTypesCheckOptionsCallback =
+            forkedTypeScriptTypesCheckOptionsCallback;
+    }
+
     enableVueLoader(vueLoaderOptionsCallback = () => {}) {
         this.useVueLoader = true;
 

lib/loader-features.js

@@ -41,6 +41,11 @@ const loaderFeatures = {
         packages: ['typescript', 'ts-loader'],
         description: 'process TypeScript files'
     },
+    forkedtypecheck: {
+        method: 'enableForkedTypeScriptTypesChecking()',
+        packages: ['typescript', 'ts-loader', 'fork-ts-checker-webpack-plugin'],
+        description: 'check TypeScript types in a separate process'
+    },
     vue: {
         method: 'enableVueLoader()',
         // vue is needed so the end-user can do things

lib/loaders/typescript.js

@@ -32,6 +32,17 @@ module.exports = {
             [config]
         );
 
+        // fork-ts-checker-webpack-plugin integration
+        if (webpackConfig.useForkedTypeScriptTypeChecking) {
+            loaderFeatures.ensureLoaderPackagesExist('forkedtypecheck');
+            // force transpileOnly to speed up
+            config.transpileOnly = true;
+
+            // add forked ts types plugin to the stack
+            const forkedTypesPluginUtil = require('../plugins/forked-ts-types'); // eslint-disable-line
+            forkedTypesPluginUtil(webpackConfig);
+        }
+
         // use ts alongside with babel
         // @see https://github.com/TypeStrong/ts-loader/blob/master/README.md#babel
         let loaders = babelLoader.getLoaders(webpackConfig);

lib/plugins/forked-ts-types.js

@@ -0,0 +1,29 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin'); // eslint-disable-line
+
+/**
+ * @param {WebpackConfig} webpackConfig
+ * @return {void}
+ */
+module.exports = function(webpackConfig) {
+    let config = {};
+
+    // allow for ts-loader config to be controlled
+    webpackConfig.forkedTypeScriptTypesCheckOptionsCallback.apply(
+        // use config as the this variable
+        config,
+        [config]
+    );
+
+    webpackConfig.addPlugin(new ForkTsCheckerWebpackPlugin(config));
+};

package.json

@@ -55,6 +55,7 @@
     "eslint": "^3.19.0",
     "eslint-plugin-header": "^1.0.0",
     "eslint-plugin-node": "^4.2.2",
+    "fork-ts-checker-webpack-plugin": "^0.2.7",
     "http-server": "^0.9.0",
     "less": "^2.7.2",
     "less-loader": "^4.0.2",

test/WebpackConfig.js

@@ -356,6 +356,25 @@ describe('WebpackConfig object', () => {
         });
     });
 
+    describe('enableForkedTypeScriptTypesChecking', () => {
+        it('Calling method sets it', () => {
+            const config = createConfig();
+            config.enableTypeScriptLoader();
+            const testCallback = () => {};
+            config.enableForkedTypeScriptTypesChecking(testCallback);
+            expect(config.forkedTypeScriptTypesCheckOptionsCallback)
+                .to.equal(testCallback);
+        });
+
+        it('Calling with non-callback throws an error', () => {
+            const config = createConfig();
+
+            expect(() => {
+                config.enableForkedTypeScriptTypesChecking('FOO');
+            }).to.throw('must be a callback function');
+        });
+    });
+
     describe('addPlugin', () => {
         it('extends the current registered plugins', () => {
             const config = createConfig();

test/functional.js

@@ -288,13 +288,13 @@ describe('Functional tests using webpack', function() {
                         'bg.483832e48e67e6a3b7f0ae064eadca51.css',
                         'manifest.json'
                     ]
-                );
+                    );
 
                 expect(path.join(config.outputPath, 'images')).to.be.a.directory()
                     .with.files([
                         'symfony_logo.ea1ca6f7.png'
                     ]
-                );
+                    );
 
                 webpackAssert.assertOutputFileContains(
                     'bg.483832e48e67e6a3b7f0ae064eadca51.css',
@@ -319,19 +319,19 @@ describe('Functional tests using webpack', function() {
                         'font.css',
                         'manifest.json'
                     ]
-                );
+                    );
 
                 expect(path.join(config.outputPath, 'images')).to.be.a.directory()
                     .with.files([
                         'symfony_logo.ea1ca6f7.png'
                     ]
-                );
+                    );
 
                 expect(path.join(config.outputPath, 'fonts')).to.be.a.directory()
                     .with.files([
                         'Roboto.9896f773.woff2'
                     ]
-                );
+                    );
 
                 webpackAssert.assertOutputFileContains(
                     'bg.css',
@@ -359,21 +359,21 @@ describe('Functional tests using webpack', function() {
                         'styles.css',
                         'manifest.json'
                     ]
-                );
+                    );
 
                 expect(path.join(config.outputPath, 'images')).to.be.a.directory()
                     .with.files([
                         'symfony_logo.ea1ca6f7.png',
                         'symfony_logo.f27119c2.png'
                     ]
-                );
+                    );
 
                 expect(path.join(config.outputPath, 'fonts')).to.be.a.directory()
                     .with.files([
                         'Roboto.9896f773.woff2',
                         'Roboto.3c37aa69.woff2'
                     ]
-                );
+                    );
 
                 webpackAssert.assertOutputFileContains(
                     'styles.css',
@@ -587,7 +587,7 @@ module.exports = {
 
             fs.writeFileSync(
                 path.join(appDir, '.babelrc'),
-`
+                `
 {
   "presets": [
     ["env", {
@@ -667,6 +667,26 @@ module.exports = {
             });
         });
 
+        it('TypeScript is compiled and type checking is done in a separate process!', (done) => {
+            this.timeout(8000);
+            setTimeout(done, 7000);
+
+            const config = createWebpackConfig('www/build', 'dev');
+            config.setPublicPath('/build');
+            config.addEntry('main', ['./js/render.ts', './js/index.ts']);
+            config.enableTypeScriptLoader();
+            // test should fail if `config.tsconfig` is not set up properly
+            config.enableForkedTypeScriptTypesChecking((config) => {
+                config.silent = true; // remove to get output on terminal
+            });
+
+            expect(function() {
+                testSetup.runWebpack(config, (webpackAssert) => {
+                    done();
+                });
+            }).to.throw('wrong `tsconfig` path in fork plugin configuration (should be a relative or absolute path)');
+        });
+
         it('The output directory is cleaned between builds', (done) => {
             const config = createWebpackConfig('www/build', 'dev');
             config.setPublicPath('/build');

test/index.js

@@ -186,6 +186,15 @@ describe('Public API', () => {
 
     });
 
+    describe('enableForkedTypeScriptTypesChecking', () => {
+
+        it('must return the API object', () => {
+            const returnedValue = api.enableForkedTypeScriptTypesChecking();
+            expect(returnedValue).to.equal(api);
+        });
+
+    });
+
     describe('enableVueLoader', () => {
 
         it('must return the API object', () => {

test/plugins/forked-ts-types.js

@@ -0,0 +1,41 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const expect = require('chai').expect;
+const WebpackConfig = require('../../lib/WebpackConfig');
+const RuntimeConfig = require('../../lib/config/RuntimeConfig');
+const tsLoader = require('../../lib/loaders/typescript');
+const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
+
+function createConfig() {
+    const runtimeConfig = new RuntimeConfig();
+    runtimeConfig.context = __dirname;
+    runtimeConfig.babelRcFileExists = false;
+
+    return new WebpackConfig(runtimeConfig);
+}
+
+describe('plugins/forkedtypecheck', () => {
+    it('getPlugins() basic usage', () => {
+        const config = createConfig();
+        config.enableTypeScriptLoader();
+        config.enableForkedTypeScriptTypesChecking();
+
+        expect(config.plugins).to.have.lengthOf(0);
+        const tsTypeChecker = require('../../lib/plugins/forked-ts-types');
+        tsTypeChecker(config);
+        expect(config.plugins).to.have.lengthOf(1);
+        expect(config.plugins[0]).to.be.an.instanceof(ForkTsCheckerWebpackPlugin);
+        // after enabling plugin, check typescript loader has right config
+        const actualLoaders = tsLoader.getLoaders(config);
+        expect(actualLoaders[1].options.transpileOnly).to.be.true;
+    });
+});