Start validating loader option names #629

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## v2.3.7
+
+- [Start validating the options supplied to the loader](https://github.com/TypeStrong/ts-loader/pull/630) (#629) - thanks @johnnyreilly!
+
 ## v2.3.6
 
 - [Fix kills ts-loader dependant builds issue](https://github.com/TypeStrong/ts-loader/pull/627) (#626) - thanks @Loilo!

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ts-loader",
-  "version": "2.3.6",
+  "version": "2.3.7",
   "description": "TypeScript loader for webpack",
   "main": "index.js",
   "scripts": {

src/index.ts

@@ -61,8 +61,8 @@ function successLoader(
 
     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"
-            : "";
+            ? '\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}`);
     }
 
@@ -90,15 +90,48 @@ function getLoaderOptions(loader: Webpack) {
         webpackIndex = webpackInstances.push(loader._compiler) - 1;
     }
 
-    const queryOptions = loaderUtils.getOptions<LoaderOptions>(loader) || {} as LoaderOptions;
+    const loaderOptions = loaderUtils.getOptions<LoaderOptions>(loader) || {} as LoaderOptions;
     const configFileOptions: PartialLoaderOptions = loader.options.ts || {};
 
-    const instanceName = webpackIndex + '_' + (queryOptions.instance || configFileOptions.instance || 'default');
+    const instanceName = webpackIndex + '_' + (loaderOptions.instance || configFileOptions.instance || 'default');
 
     if (hasOwnProperty(loaderOptionsCache, instanceName)) {
         return loaderOptionsCache[instanceName];
     }
 
+    validateLoaderOptions(loaderOptions);
+
+    const options = makeLoaderOptions(instanceName, configFileOptions, loaderOptions);
+
+    loaderOptionsCache[instanceName] = options;
+
+    return options;
+}
+
+type ValidLoaderOptions = keyof LoaderOptions;
+const validLoaderOptions: ValidLoaderOptions[] = ['silent', 'logLevel', 'logInfoToStdOut', 'instance', 'compiler', 'configFile', 'configFileName' /*DEPRECATED*/, 'transpileOnly', 'ignoreDiagnostics', 'visualStudioErrorFormat', 'compilerOptions', 'appendTsSuffixTo', 'appendTsxSuffixTo', 'entryFileIsJs', 'happyPackMode', 'getCustomTransformers'];
+
+/**
+ * Validate the supplied loader options.
+ * At present this validates the option names only; in future we may look at validating the values too
+ * @param loaderOptions 
+ */
+function validateLoaderOptions(loaderOptions: LoaderOptions) {
+    const loaderOptionKeys = Object.keys(loaderOptions);
+    for (let i = 0; i < loaderOptionKeys.length; i++) {
+        const option = loaderOptionKeys[i];
+        const isUnexpectedOption = (validLoaderOptions as string[]).indexOf(option) === -1;
+        if (isUnexpectedOption) {
+            throw new Error(`ts-loader was supplied with an unexpected loader option: ${option}
+
+Please take a look at the options you are supplying; the following are valid options:
+${ validLoaderOptions.join(' / ')}
+`);
+        }
+    }
+}
+
+function makeLoaderOptions(instanceName: string, configFileOptions: Partial<LoaderOptions>, loaderOptions: LoaderOptions) {
     const options = Object.assign({}, {
         silent: false,
         logLevel: 'INFO',
@@ -113,14 +146,14 @@ function getLoaderOptions(loader: Webpack) {
         transformers: {},
         entryFileIsJs: false,
         happyPackMode: false,
-    }, configFileOptions, queryOptions);
+    }, configFileOptions, loaderOptions);
 
     // Use deprecated `configFileName` as fallback for `configFile`
-    if (queryOptions.configFileName) {
-        if (queryOptions.configFile) {
+    if (loaderOptions.configFileName) {
+        if (loaderOptions.configFile) {
             throw new Error('ts-loader options `configFile` and `configFileName` are mutually exclusive');
         } else {
-            options.configFile = queryOptions.configFileName;
+            options.configFile = loaderOptions.configFileName;
         }
     }
 
@@ -131,8 +164,6 @@ function getLoaderOptions(loader: Webpack) {
     // happypack can be used only together with transpileOnly mode
     options.transpileOnly = options.happyPackMode ? true : options.transpileOnly;
 
-    loaderOptionsCache[instanceName] = options;
-
     return options;
 }
 

test/comparison-tests/create-and-execute-test.js

@@ -362,7 +362,7 @@ function getNormalisedFileContent(file, location, test) {
     var filePath = path.join(location, file);
     try {
         var originalContent = fs.readFileSync(filePath).toString();
-        fileContent = (file.indexOf('output.') === 0)
+        fileContent = (file.indexOf('output.') === 0
             ? normaliseString(originalContent)
                 // We don't want a difference in the number of kilobytes to fail the build
                 .replace(/[\d]+[.][\d]* kB/g, ' A-NUMBER-OF kB')
@@ -374,10 +374,13 @@ function getNormalisedFileContent(file, location, test) {
                 .replace(/(\(dist[\/|\\]\w*.js:)(\d*)(:)(\d*)(\))/g, function(match, openingBracketPathAndColon, lineNumber, colon, columnNumber, closingBracket){
                     return openingBracketPathAndColon + 'irrelevant-line-number' + colon + 'irrelevant-column-number' + closingBracket;
                 })
-            : normaliseString(originalContent);
-    }
-    catch (e) {
-        fileContent = '!!!' + filePath + ' doesnt exist!!!';
+            : normaliseString(originalContent))
+            // Ignore 'at C:/source/ts-loader/dist/index.js:90:19' style row number / column number differences
+            .replace(/at (.*)(dist[\/|\\]\w*.js:)(\d*)(:)(\d*)/g, function(match, spaceAndStartOfPath, remainingPathAndColon, lineNumber, colon, columnNumber){
+                return 'at ' + remainingPathAndColon + 'irrelevant-line-number' + colon + 'irrelevant-column-number';
+            });
+    } catch (e) {
+        fileContent = '!!!' + filePath + ' doePsnt exist!!!';
     }
     return fileContent;
 }

test/comparison-tests/validateLoaderOptionNames/app.ts

@@ -0,0 +1 @@
+var a = 0; 

test/comparison-tests/validateLoaderOptionNames/expectedOutput-2.5/bundle.js

@@ -0,0 +1,76 @@
+/******/ (function(modules) { // webpackBootstrap
+/******/ 	// The module cache
+/******/ 	var installedModules = {};
+/******/
+/******/ 	// The require function
+/******/ 	function __webpack_require__(moduleId) {
+/******/
+/******/ 		// Check if module is in cache
+/******/ 		if(installedModules[moduleId]) {
+/******/ 			return installedModules[moduleId].exports;
+/******/ 		}
+/******/ 		// Create a new module (and put it into the cache)
+/******/ 		var module = installedModules[moduleId] = {
+/******/ 			i: moduleId,
+/******/ 			l: false,
+/******/ 			exports: {}
+/******/ 		};
+/******/
+/******/ 		// Execute the module function
+/******/ 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
+/******/
+/******/ 		// Flag the module as loaded
+/******/ 		module.l = true;
+/******/
+/******/ 		// Return the exports of the module
+/******/ 		return module.exports;
+/******/ 	}
+/******/
+/******/
+/******/ 	// expose the modules object (__webpack_modules__)
+/******/ 	__webpack_require__.m = modules;
+/******/
+/******/ 	// expose the module cache
+/******/ 	__webpack_require__.c = installedModules;
+/******/
+/******/ 	// identity function for calling harmony imports with the correct context
+/******/ 	__webpack_require__.i = function(value) { return value; };
+/******/
+/******/ 	// define getter function for harmony exports
+/******/ 	__webpack_require__.d = function(exports, name, getter) {
+/******/ 		if(!__webpack_require__.o(exports, name)) {
+/******/ 			Object.defineProperty(exports, name, {
+/******/ 				configurable: false,
+/******/ 				enumerable: true,
+/******/ 				get: getter
+/******/ 			});
+/******/ 		}
+/******/ 	};
+/******/
+/******/ 	// getDefaultExport function for compatibility with non-harmony modules
+/******/ 	__webpack_require__.n = function(module) {
+/******/ 		var getter = module && module.__esModule ?
+/******/ 			function getDefault() { return module['default']; } :
+/******/ 			function getModuleExports() { return module; };
+/******/ 		__webpack_require__.d(getter, 'a', getter);
+/******/ 		return getter;
+/******/ 	};
+/******/
+/******/ 	// Object.prototype.hasOwnProperty.call
+/******/ 	__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };
+/******/
+/******/ 	// __webpack_public_path__
+/******/ 	__webpack_require__.p = "";
+/******/
+/******/ 	// Load entry module and return exports
+/******/ 	return __webpack_require__(__webpack_require__.s = 0);
+/******/ })
+/************************************************************************/
+/******/ ([
+/* 0 */
+/***/ (function(module, exports) {
+
+throw new Error("Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption\n\nPlease take a look at the options you are supplying; the following are valid options:\nsilent / logLevel / logInfoToStdOut / instance / compiler / configFile / configFileName / transpileOnly / ignoreDiagnostics / visualStudioErrorFormat / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / entryFileIsJs / happyPackMode / getCustomTransformers\n\n    at validateLoaderOptions (C:/source/ts-loader/dist/index.js:92:19)\n    at getLoaderOptions (C:/source/ts-loader/dist/index.js:75:5)\n    at Object.loader (C:/source/ts-loader/dist/index.js:23:19)");
+
+/***/ })
+/******/ ]);

test/comparison-tests/validateLoaderOptionNames/expectedOutput-2.5/output.txt

@@ -0,0 +1,14 @@
+    Asset     Size  Chunks             Chunk Names
+bundle.js  3.32 kB       0  [emitted]  main
+chunk    {0} bundle.js (main) 687 bytes [entry] [rendered]
+    [0] ./.test/validateLoaderOptionNames/app.ts 687 bytes {0} [built] [failed] [1 error]
+
+ERROR in ./.test/validateLoaderOptionNames/app.ts
+Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption
+
+Please take a look at the options you are supplying; the following are valid options:
+silent / logLevel / logInfoToStdOut / instance / compiler / configFile / configFileName / transpileOnly / ignoreDiagnostics / visualStudioErrorFormat / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / entryFileIsJs / happyPackMode / getCustomTransformers
+
+    at validateLoaderOptions (dist\index.js:92:19)
+    at getLoaderOptions (dist\index.js:75:5)
+    at Object.loader (dist\index.js:23:19)

test/comparison-tests/validateLoaderOptionNames/tsconfig.json

@@ -0,0 +1,4 @@
+{
+	"compilerOptions": {
+	}
+}

test/comparison-tests/validateLoaderOptionNames/webpack.config.js

@@ -0,0 +1,16 @@
+module.exports = {
+    entry: './app.ts',
+    output: {
+        filename: 'bundle.js'
+    },
+    resolve: {
+        extensions: ['.ts', '.js']
+    },
+    module: {
+        rules: [
+            { test: /\.ts$/, loader: 'ts-loader', options: { notRealOption: true } }
+        ]
+    }
+}
+
+