Ensure a separate webpack instance is created for different loader options (#1104)

appzuka · web-flow · commit 1b84fed767c0 · 2020-05-11T16:14:05.000+01:00
* Ensure a separate webpack instance is created if different loader options are used
Add execution test for this behaviour

* Fix import order (lint error)

* Update version to 7.04 and include in changelog

* Tweak to Changelog
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## v7.0.4
+* [Ensure a separate webpack instance is created for different loader options](https://github.com/TypeStrong/ts-loader/pull/1104) - thanks @appzuka
+
 ## v7.0.3
 * [Ensure that JSON files are included in build module resolution](https://github.com/TypeStrong/ts-loader/pull/1101) - thanks @berickson1
 
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "ts-loader",
-  "version": "7.0.3",
+  "version": "7.0.4",
   "description": "TypeScript loader for webpack",
   "main": "index.js",
   "types": "dist",
diff --git a/src/index.ts b/src/index.ts
@@ -1,3 +1,4 @@
+import * as crypto from 'crypto';
 import * as loaderUtils from 'loader-utils';
 import * as path from 'path';
 import * as typescript from 'typescript';
@@ -220,6 +221,22 @@ function setModuleMeta(
   }
 }
 
+/**
+ * Get a unique hash based on the contents of the options
+ * Hash is created from the values converted to strings
+ * Values which are functions (such as getCustomTransformers) are
+ * converted to strings by this code, which JSON.stringify would not do.
+ */
+function getOptionsHash(loaderOptions: LoaderOptions) {
+  const hash = crypto.createHash('sha256');
+  Object.values(loaderOptions).map((v: any) => {
+    if (v) {
+      hash.update(v.toString());
+    }
+  });
+  return hash.digest('hex').substring(0, 16);
+}
+
 /**
  * either retrieves loader options from the cache
  * or creates them, adds them to the cache and returns
@@ -235,8 +252,12 @@ function getLoaderOptions(loaderContext: webpack.loader.LoaderContext) {
     loaderUtils.getOptions<LoaderOptions>(loaderContext) ||
     ({} as LoaderOptions);
 
+  // If no instance name is given in the options, use the hash of the loader options
+  // In this way, if different options are given the instances will be different
   const instanceName =
-    webpackIndex + '_' + (loaderOptions.instance || 'default');
+    webpackIndex +
+    '_' +
+    (loaderOptions.instance || 'default_' + getOptionsHash(loaderOptions));
 
   if (!loaderOptionsCache.hasOwnProperty(instanceName)) {
     loaderOptionsCache[instanceName] = new WeakMap();
@@ -528,7 +549,7 @@ function getEmit(
 
   loaderContext._module.buildMeta.tsLoaderDefinitionFileVersions = dependencies.map(
     defFilePath =>
-      path.relative(loaderContext.rootContext,defFilePath) +
+      path.relative(loaderContext.rootContext, defFilePath) +
       '@' +
       (
         instance.files.get(defFilePath) ||
diff --git a/test/execution-tests/loaderOptions/Readme.md b/test/execution-tests/loaderOptions/Readme.md
@@ -0,0 +1,3 @@
+## Loader Options Test
+
+This test includes 3 submodules, only 1 of which should have the transformer applied via the options.  This tests that separate webpack instances are created when module options are different.
diff --git a/test/execution-tests/loaderOptions/karma.conf.js b/test/execution-tests/loaderOptions/karma.conf.js
@@ -0,0 +1,17 @@
+/* eslint-disable no-var, strict */
+'use strict';
+var webpackConfig = require('./webpack.config.js');
+var makeKarmaConfig = require('../../karmaConfig');
+
+module.exports = function(config) {
+  config.set(
+    makeKarmaConfig({
+      config,
+      webpackConfig,
+      files: [
+        // This ensures we have the es6 shims in place from babel and then loads all the tests
+        'main.js'
+      ]
+    })
+  );
+};
diff --git a/test/execution-tests/loaderOptions/lib/externalLib.d.ts b/test/execution-tests/loaderOptions/lib/externalLib.d.ts
@@ -0,0 +1,7 @@
+declare module externalLib {
+  export function doSomething(arg: any): void;
+}
+
+declare module 'externalLib' {
+    export = externalLib
+}
diff --git a/test/execution-tests/loaderOptions/lib/externalLib.js b/test/execution-tests/loaderOptions/lib/externalLib.js
@@ -0,0 +1,3 @@
+module.exports = {
+    doSomething: function() { }   
+}
diff --git a/test/execution-tests/loaderOptions/main.js b/test/execution-tests/loaderOptions/main.js
@@ -0,0 +1,2 @@
+﻿const testsContext = require.context('./', true, /\.tests\.ts(x?)$/);
+testsContext.keys().forEach(testsContext);
diff --git a/test/execution-tests/loaderOptions/package.json b/test/execution-tests/loaderOptions/package.json
@@ -0,0 +1,10 @@
+{
+  "name": "basic",
+  "license": "MIT",
+  "version": "1.0.0",
+  "main": "index.js",
+  "devDependencies": {
+    "@types/jasmine": "^2.5.35",
+    "jasmine-core": "^2.3.4"
+  }
+}
diff --git a/test/execution-tests/loaderOptions/src/app.ts b/test/execution-tests/loaderOptions/src/app.ts
@@ -0,0 +1,3 @@
+import submodule = require('./submodule/submodule');
+import externalLib = require('externalLib');
+externalLib.doSomething(submodule);
diff --git a/test/execution-tests/loaderOptions/src/submodule/submodule.ts b/test/execution-tests/loaderOptions/src/submodule/submodule.ts
@@ -0,0 +1,5 @@
+import externalLib = require('externalLib');
+
+externalLib.doSomething("");
+var message = "Hello from submodule"
+export = message
diff --git a/test/execution-tests/loaderOptions/src/submodule/submodule2.ts b/test/execution-tests/loaderOptions/src/submodule/submodule2.ts
@@ -0,0 +1,5 @@
+import externalLib = require('externalLib');
+
+externalLib.doSomething("");
+var message = "Hello from submodule2"
+export = message
diff --git a/test/execution-tests/loaderOptions/src/submodule/submodule3.ts b/test/execution-tests/loaderOptions/src/submodule/submodule3.ts
@@ -0,0 +1,5 @@
+import externalLib = require('externalLib');
+
+externalLib.doSomething("");
+var message = "Hello from submodule3"
+export = message
diff --git a/test/execution-tests/loaderOptions/test/app.tests.ts b/test/execution-tests/loaderOptions/test/app.tests.ts
@@ -0,0 +1,22 @@
+import submodule = require('../src/submodule/submodule');
+import submodule2 = require('../src/submodule/submodule2');
+import submodule3 = require('../src/submodule/submodule3');
+import externalLib = require('externalLib');
+
+describe("app", () => {
+  it("externalLib can be called", () => {
+    expect(externalLib.doSomething(submodule)).toBeUndefined();
+  });
+
+  it("submodule return value should not be transformed", () => {
+    expect(submodule).toBe("Hello "+"from submodule");
+  });
+
+  it("submodule2 return value should be transformed", () => {
+    expect(submodule2).toBe("HELLO "+"FROM SUBMODULE2");
+  });
+  
+  it("submodule3 return value should not be transformed", () => {
+    expect(submodule3).toBe("Hello "+"from submodule3");
+  });
+});
diff --git a/test/execution-tests/loaderOptions/tsconfig.json b/test/execution-tests/loaderOptions/tsconfig.json
@@ -0,0 +1,5 @@
+{
+	"compilerOptions": {
+		"noEmitOnError": true
+	}
+}
diff --git a/test/execution-tests/loaderOptions/uppercaseStringLiteralTransformer.js b/test/execution-tests/loaderOptions/uppercaseStringLiteralTransformer.js
@@ -0,0 +1,18 @@
+"use strict";
+exports.__esModule = true;
+var ts = require("typescript");
+var transformer = function (context) {
+    var visitor = function (node) {
+        if (node.kind === ts.SyntaxKind.StringLiteral) {
+            var text = node.text;
+            if (text.match(/^Hello from submodule/) !== null) {
+                if (text !== text.toUpperCase()) {
+                    return ts.createLiteral(text.toUpperCase());
+                }
+            }
+        }
+        return ts.visitEachChild(node, visitor, context);
+    };
+    return function (node) { return ts.visitNode(node, visitor); };
+};
+exports["default"] = transformer;
diff --git a/test/execution-tests/loaderOptions/webpack.config.js b/test/execution-tests/loaderOptions/webpack.config.js
@@ -0,0 +1,40 @@
+var path = require('path')
+
+var uppercaseStringLiteralTransformer = require('./uppercaseStringLiteralTransformer').default;
+
+module.exports = {
+    mode: 'development',
+    entry: './src/app.ts',
+    output: {
+        filename: 'bundle.js'
+    },
+    resolve: {
+        alias: { externalLib: path.join(__dirname, "./lib/externalLib.js") },
+        extensions: ['.ts', '.js']
+    },
+    module: {
+        rules: [
+            {
+                test: /submodule3\.ts$/,
+                loader: 'ts-loader',
+            },
+            {
+                test: /submodule2\.ts$/,
+                loader: 'ts-loader',
+                options: {
+                    getCustomTransformers: (program) => ({
+                        before: [uppercaseStringLiteralTransformer]
+                    })
+                },
+            },
+            {
+                test: /\.ts$/,
+                exclude: /submodule\d\.ts$/,
+                loader: 'ts-loader',
+            }
+        ]
+    }
+}
+
+// for test harness purposes only, you would not need this in a normal project
+module.exports.resolveLoader = { alias: { 'ts-loader': require('path').join(__dirname, "../../../index.js") } }
diff --git a/test/execution-tests/loaderOptions/yarn.lock b/test/execution-tests/loaderOptions/yarn.lock
@@ -0,0 +1,11 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+"@types/jasmine@^2.5.35":
+  version "2.8.5"
+  resolved "https://registry.yarnpkg.com/@types/jasmine/-/jasmine-2.8.5.tgz#96e58872583fa80c7ea0dd29024b180d5e133678"
+
+jasmine-core@^2.3.4:
+  version "2.9.1"
+  resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-2.9.1.tgz#b6bbc1d8e65250d56f5888461705ebeeeb88f22f"

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "ts-loader",`
`3`		`- "version": "7.0.3",`
	`3`	`+ "version": "7.0.4",`
`4`	`4`	`"description": "TypeScript loader for webpack",`
`5`	`5`	`"main": "index.js",`
`6`	`6`	`"types": "dist",`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+## Loader Options Test`
	`2`	`+`
	`3`	`+This test includes 3 submodules, only 1 of which should have the transformer applied via the options. This tests that separate webpack instances are created when module options are different.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+module.exports = {`
	`2`	`+ doSomething: function() { }`
	`3`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+const testsContext = require.context('./', true, /\.tests\.ts(x?)$/);`
	`2`	`+testsContext.keys().forEach(testsContext);`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import submodule = require('./submodule/submodule');`
	`2`	`+import externalLib = require('externalLib');`
	`3`	`+externalLib.doSomething(submodule);`