Merge pull request #412 from johnnyreilly/master

minor perf optimisation for loaderoptions
TypeStrong · Dec 15, 2016 · a4aef4c · a4aef4c
2 parents 5bb2a4a + f0d0ad9
commit a4aef4c
Show file tree

Hide file tree

Showing 15 changed files with 3,312 additions and 560 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## v1.3.3 - not released yet
+
+- [Fix bug when "extend"ing a tsconfig that specifies "allowJs"](https://github.com/TypeStrong/ts-loader/pull/415) THanks @cspotcode
+
 ## v1.3.2
 
 - [Upgrade enhanced-resolve to v3](https://github.com/TypeStrong/ts-loader/pull/411)

diff --git a/package.json b/package.json
@@ -57,7 +57,7 @@
     "mocha": "^3.1.0",
     "phantomjs-prebuilt": "^2.1.2",
     "rimraf": "^2.4.2",
-    "typescript": "^2.0.3",
+    "typescript": "^2.1.4",
     "typings": "^2.0.0",
     "vue": "^2.0.5",
     "vue-loader": "^9.7.0",

diff --git a/src/after-compile.ts b/src/after-compile.ts
@@ -2,6 +2,7 @@ import interfaces = require('./interfaces');
 import path = require('path');
 import typescript = require('typescript');
 import utils = require('./utils');
+import constants = require('./constants');
 
 function makeAfterCompile(
     instance: interfaces.TSInstance,
@@ -51,8 +52,8 @@ function provideCompilerOptionDiagnosticErrorsToWebpack(
     instance: interfaces.TSInstance,
     configFilePath: string
 ) {
-    const { languageService, loaderOptions, compiler } = instance;
     if (getCompilerOptionDiagnostics) {
+        const { languageService, loaderOptions, compiler } = instance;
         utils.registerWebpackErrors(
             compilation.errors,
             utils.formatErrors(
@@ -125,7 +126,7 @@ function provideErrorsToWebpack(
 ) {
     const { compiler, languageService, files, loaderOptions } = instance;
     Object.keys(filesToCheckForErrors)
-        .filter(filePath => !!filePath.match(/(\.d)?\.ts(x?)$/))
+        .filter(filePath => !!filePath.match(constants.dtsTsTsxRegex))
         .forEach(filePath => {
             const errors = languageService.getSyntacticDiagnostics(filePath).concat(languageService.getSemanticDiagnostics(filePath));
             if (errors.length > 0) {
@@ -161,10 +162,10 @@ function provideDeclarationFilesToWebpack(
     compilation: interfaces.WebpackCompilation
 ) {
     Object.keys(filesToCheckForErrors)
-        .filter(filePath => !!filePath.match(/\.ts(x?)$/))
+        .filter(filePath => !!filePath.match(constants.tsTsxRegex))
         .forEach(filePath => {
             const output = languageService.getEmitOutput(filePath);
-            const declarationFile = output.outputFiles.filter(fp => !!fp.name.match(/\.d.ts$/)).pop();
+            const declarationFile = output.outputFiles.filter(outputFile => !!outputFile.name.match(constants.dtsDtsxRegex)).pop();
             if (declarationFile) {
                 const assetPath = path.relative(compilation.compiler.context, declarationFile.name);
                 compilation.assets[assetPath] = {

diff --git a/src/compilerSetup.ts b/src/compilerSetup.ts
@@ -46,7 +46,7 @@ export function getCompilerOptions(
     compiler: typeof typescript,
     configParseResult: typescript.ParsedCommandLine
 ) {
-    const compilerOptions = objectAssign<typescript.CompilerOptions>({}, configParseResult.options, {
+    const compilerOptions = objectAssign({}, configParseResult.options, {
         skipDefaultLibCheck: true,
         suppressOutputPathCheck: true, // This is why: https://github.com/Microsoft/TypeScript/issues/7363
     });

diff --git a/src/constants.ts b/src/constants.ts
@@ -10,4 +10,11 @@ export const LineFeedCode = 1;
 export const ScriptTargetES2015 = 2;
 
 export const ModuleKindNone = 0;
-export const ModuleKindCommonJs = 1;
+export const ModuleKindCommonJs = 1;
+
+export const tsTsxRegex = /\.ts(x?)$/i;
+export const dtsDtsxRegex = /\.d\.ts(x?)$/i;
+export const dtsTsTsxRegex = /(\.d)?\.ts(x?)$/i;
+export const tsTsxJsJsxRegex = /\.tsx?$|\.jsx?$/i;
+export const jsJsx = /\.js(x?)$/i;
+export const jsJsxMap = /\.js(x?)\.map$/i;
diff --git a/src/index.ts b/src/index.ts
@@ -6,34 +6,40 @@ require('colors');
 import instances = require('./instances');
 import interfaces = require('./interfaces');
 import utils = require('./utils');
+import constants = require('./constants');
 
-let webpackInstances: any = [];
-const definitionFileRegex = /\.d\.ts$/;
+const webpackInstances: interfaces.Compiler[] = [];
+const loaderOptionsCache: interfaces.LoaderOptionsCache = {};
 
+type PartialLoaderOptions = interfaces.Partial<interfaces.LoaderOptions>;
+
+/**
+ * The entry point for ts-loader
+ */
 function loader(this: interfaces.Webpack, contents: string) {
     this.cacheable && this.cacheable();
     const callback = this.async();
-    const options = makeOptions(this);
-    const rawFilePath = path.normalize(this.resourcePath);
-    const filePath = utils.appendTsSuffixIfMatch(options.appendTsSuffixTo, rawFilePath);
+    const options = getLoaderOptions(this);
 
-    const { instance, error } = instances.ensureTypeScriptInstance(options, this);
+    const { instance, error } = instances.getTypeScriptInstance(options, this);
 
     if (error) {
         callback(error);
         return;
     }
 
-    const file = updateFileInCache(filePath, contents, instance);
+    const rawFilePath = path.normalize(this.resourcePath);
+    const filePath = utils.appendTsSuffixIfMatch(options.appendTsSuffixTo, rawFilePath);
+    const fileVersion = updateFileInCache(filePath, contents, instance);
 
     const { outputText, sourceMapText } = options.transpileOnly
         ? getTranspilationEmit(filePath, contents, instance, this)
         : getEmit(filePath, instance, this);
 
     if (outputText === null || outputText === undefined) {
-        const additionalGuidance = filePath.indexOf('node_modules') !== -1 
-        ? "\nYou should not need to recompile .ts files in node_modules.\nPlease contact the package author to advise them to use --declaration --outDir.\nMore https://github.com/Microsoft/TypeScript/issues/12358"
-        : "";
+        const additionalGuidance = filePath.indexOf('node_modules') !== -1
+            ? "\nYou should not need to recompile .ts files in node_modules.\nPlease contact the package author to advise them to use --declaration --outDir.\nMore https://github.com/Microsoft/TypeScript/issues/12358"
+            : "";
         throw new Error(`Typescript emitted no output for ${filePath}.${additionalGuidance}`);
     }
 
@@ -42,20 +48,35 @@ function loader(this: interfaces.Webpack, contents: string) {
     // Make sure webpack is aware that even though the emitted JavaScript may be the same as
     // a previously cached version the TypeScript may be different and therefore should be
     // treated as new
-    this._module.meta.tsLoaderFileVersion = file.version;
+    this._module.meta.tsLoaderFileVersion = fileVersion;
 
     callback(null, output, sourceMap);
 }
 
-function makeOptions(loader: interfaces.Webpack) {
+/**
+ * either retrieves loader options from the cache
+ * or creates them, adds them to the cache and returns
+ */
+function getLoaderOptions(loader: interfaces.Webpack) {
+    // differentiate the TypeScript instance based on the webpack instance
+    let webpackIndex = webpackInstances.indexOf(loader._compiler);
+    if (webpackIndex === -1) {
+        webpackIndex = webpackInstances.push(loader._compiler) - 1;
+    }
+
     const queryOptions = loaderUtils.parseQuery<interfaces.LoaderOptions>(loader.query);
-    const configFileOptions = loader.options.ts || {};
+    const configFileOptions: PartialLoaderOptions = loader.options.ts || {};
+
+    const instanceName = webpackIndex + '_' + (queryOptions.instance || configFileOptions.instance || 'default');
+
+    if (utils.hasOwnProperty(loaderOptionsCache, instanceName)) {
+        return loaderOptionsCache[instanceName];
+    }
 
-    const options = objectAssign<interfaces.LoaderOptions>({}, {
+    const options = objectAssign({}, {
         silent: false,
         logLevel: 'INFO',
         logInfoToStdOut: false,
-        instance: 'default',
         compiler: 'typescript',
         configFileName: 'tsconfig.json',
         transpileOnly: false,
@@ -64,24 +85,25 @@ function makeOptions(loader: interfaces.Webpack) {
         appendTsSuffixTo: [],
         entryFileIsJs: false,
     }, configFileOptions, queryOptions);
+
     options.ignoreDiagnostics = utils.arrify(options.ignoreDiagnostics).map(Number);
     options.logLevel = options.logLevel.toUpperCase();
+    options.instance = instanceName;
 
-    // differentiate the TypeScript instance based on the webpack instance
-    let webpackIndex = webpackInstances.indexOf(loader._compiler);
-    if (webpackIndex === -1) {
-        webpackIndex = webpackInstances.push(loader._compiler) - 1;
-    }
-    options.instance = webpackIndex + '_' + options.instance;
+    loaderOptionsCache[instanceName] = options;
 
     return options;
 }
 
+/**
+ * Either add file to the overall files cache or update it in the cache when the file contents have changed
+ * Also add the file to the modified files
+ */
 function updateFileInCache(filePath: string, contents: string, instance: interfaces.TSInstance) {
     // Update file contents
     let file = instance.files[filePath];
     if (!file) {
-        file = instance.files[filePath] = <interfaces.TSFile> { version: 0 };
+        file = instance.files[filePath] = <interfaces.TSFile>{ version: 0 };
     }
 
     if (file.text !== contents) {
@@ -95,7 +117,7 @@ function updateFileInCache(filePath: string, contents: string, instance: interfa
         instance.modifiedFiles = {};
     }
     instance.modifiedFiles[filePath] = file;
-    return file;
+    return file.version;
 }
 
 function getEmit(
@@ -106,12 +128,14 @@ function getEmit(
     // Emit Javascript
     const output = instance.languageService.getEmitOutput(filePath);
 
-    // Make this file dependent on *all* definition files in the program
     loader.clearDependencies();
     loader.addDependency(filePath);
 
-    const allDefinitionFiles = Object.keys(instance.files).filter(fp => definitionFileRegex.test(fp));
-    allDefinitionFiles.forEach(loader.addDependency.bind(loader));
+    const allDefinitionFiles = Object.keys(instance.files).filter(defFilePath => !!defFilePath.match(constants.dtsDtsxRegex));
+
+    // Make this file dependent on *all* definition files in the program
+    const addDependency = loader.addDependency.bind(loader);
+    allDefinitionFiles.forEach(addDependency);
 
     /* - alternative approach to the below which is more correct but has a heavy performance cost
          see https://github.com/TypeStrong/ts-loader/issues/393
@@ -125,38 +149,43 @@ function getEmit(
     // Additionally make this file dependent on all imported files
     const additionalDependencies = instance.dependencyGraph[filePath];
     if (additionalDependencies) {
-        additionalDependencies.forEach(loader.addDependency.bind(loader));
+        additionalDependencies.forEach(addDependency);
     }
 
     loader._module.meta.tsLoaderDefinitionFileVersions = allDefinitionFiles
         .concat(additionalDependencies)
-        .map(fp => fp + '@' + (instance.files[fp] || {version: '?'}).version);
+        .map(defFilePath => defFilePath + '@' + (instance.files[defFilePath] || { version: '?' }).version);
 
-    const outputFile = output.outputFiles.filter(f => !!f.name.match(/\.js(x?)$/)).pop();
+    const outputFile = output.outputFiles.filter(outputFile => !!outputFile.name.match(constants.jsJsx)).pop();
     const outputText = (outputFile) ? outputFile.text : undefined;
 
-    const sourceMapFile = output.outputFiles.filter(f => !!f.name.match(/\.js(x?)\.map$/)).pop();
+    const sourceMapFile = output.outputFiles.filter(outputFile => !!outputFile.name.match(constants.jsJsxMap)).pop();
     const sourceMapText = (sourceMapFile) ? sourceMapFile.text : undefined;
 
     return { outputText, sourceMapText };
 }
 
+/**
+ * Transpile file
+ */
 function getTranspilationEmit(
     filePath: string,
     contents: string,
     instance: interfaces.TSInstance,
     loader: interfaces.Webpack
 ) {
     const fileName = path.basename(filePath);
-    const transpileResult = instance.compiler.transpileModule(contents, {
+
+    const { outputText, sourceMapText, diagnostics } = instance.compiler.transpileModule(contents, {
         compilerOptions: instance.compilerOptions,
         reportDiagnostics: true,
         fileName,
     });
 
-    const { outputText, sourceMapText, diagnostics } = transpileResult;
-
-    utils.registerWebpackErrors(loader._module.errors, utils.formatErrors(diagnostics, instance.loaderOptions, instance.compiler, {module: loader._module}));
+    utils.registerWebpackErrors(
+        loader._module.errors,
+        utils.formatErrors(diagnostics, instance.loaderOptions, instance.compiler, { module: loader._module })
+    );
 
     return { outputText, sourceMapText };
 }
@@ -172,14 +201,13 @@ function makeSourceMap(
         return { output: outputText, sourceMap: undefined as interfaces.SourceMap };
     }
 
-    const sourceMap = JSON.parse(sourceMapText);
-    sourceMap.sources = [loaderUtils.getRemainingRequest(loader)];
-    sourceMap.file = filePath;
-    sourceMap.sourcesContent = [contents];
-
     return {
         output: outputText.replace(/^\/\/# sourceMappingURL=[^\r\n]*/gm, ''),
-        sourceMap
+        sourceMap: objectAssign(JSON.parse(sourceMapText), {
+            sources: [loaderUtils.getRemainingRequest(loader)],
+            file: filePath,
+            sourcesContent: [contents]
+        })
     };
 }
 

diff --git a/src/instances.ts b/src/instances.ts
@@ -20,7 +20,7 @@ const instances = <interfaces.TSInstances> {};
  * or returns the existing one. Multiple instances are possible by using the
  * `instance` property.
  */
-export function ensureTypeScriptInstance(
+export function getTypeScriptInstance(
     loaderOptions: interfaces.LoaderOptions,
     loader: interfaces.Webpack
 ): { instance?: interfaces.TSInstance, error?: interfaces.WebpackError } {
@@ -72,19 +72,19 @@ export function ensureTypeScriptInstance(
     }
 
     // Load initial files (core lib files, any files specified in tsconfig.json)
-    let filePath: string;
+    let normalizedFilePath: string;
     try {
         const filesToLoad = configParseResult.fileNames;
-        filesToLoad.forEach(fp => {
-            filePath = path.normalize(fp);
-            files[filePath] = {
-                text: fs.readFileSync(filePath, 'utf-8'),
+        filesToLoad.forEach(filePath => {
+            normalizedFilePath = path.normalize(filePath);
+            files[normalizedFilePath] = {
+                text: fs.readFileSync(normalizedFilePath, 'utf-8'),
                 version: 0
             };
           });
     } catch (exc) {
         return { error: utils.makeError({
-            rawMessage: `A file specified in tsconfig.json could not be found: ${ filePath }`
+            rawMessage: `A file specified in tsconfig.json could not be found: ${ normalizedFilePath }`
         }) };
     }