Merge pull request #399 from johnnyreilly/master

Introduce meaningful error when importing TypeScript

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v1.3.0
+
+- [Introduce meaningful error when importing TypeScript from `node_modules`](https://github.com/TypeStrong/ts-loader/pull/399)
+- [Introduce `entryFileIsJs` loader option which allows having an entry file which is js.](https://github.com/TypeStrong/ts-loader/pull/399) resolves #388 and #401 - thanks @Wykks and @pqr.  
+
+NB Previously the `entryFileIsJs` option was on by default when `allowJs` was true.  Now it has to be specified directly.  Strictly speaking this is a breaking change; however given this is a rarely used option which exists for what is arguably an edge case this is being added without moving to 2.0.  If this breaks people then we'll never do this again; I'd be surprised if anyone is relying on this though so we're taking a chance.  Related tests have been suffixed "-entryFileIsJs" in the test name.
+
 ## v1.2.2
 
 - [Re-exported const enums no longer break emit in watch mode](https://github.com/TypeStrong/ts-loader/pull/377) [#376] - thanks @smphhh

README.md

@@ -179,6 +179,9 @@ Advanced option to force files to go through different instances of the
 TypeScript compiler. Can be used to force segregation between different parts
 of your code.
 
+#### entryFileIsJs *(boolean) (default=false)*
+
+To be used in concert with the `allowJs` compiler option. If your entry file is JS then you'll need to set this option to true.  Please note that this is rather unusual and will generally not be necessary when using `allowJs`.
 
 #### appendTsSuffixTo *(RegExp[]) (default=[])*
 A list of regular expressions to be matched against filename. If filename matches one of the regular expressions, a `.ts` suffix will be appended to that filename.

src/index.ts

@@ -27,12 +27,15 @@ function loader(this: interfaces.Webpack, contents: string) {
 
     const file = updateFileInCache(filePath, contents, instance);
 
-    let { outputText, sourceMapText } = options.transpileOnly
+    const { outputText, sourceMapText } = options.transpileOnly
         ? getTranspilationEmit(filePath, contents, instance, this)
         : getEmit(filePath, instance, this);
 
     if (outputText === null || outputText === undefined) {
-        throw new Error(`Typescript emitted no output for ${filePath}`);
+        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}`);
     }
 
     const { sourceMap, output } = makeSourceMap(sourceMapText, outputText, filePath, contents, this);
@@ -60,6 +63,7 @@ function makeOptions(loader: interfaces.Webpack) {
         visualStudioErrorFormat: false,
         compilerOptions: {},
         appendTsSuffixTo: [],
+        entryFileIsJs: false,
     }, configFileOptions, queryOptions);
     options.ignoreDiagnostics = arrify(options.ignoreDiagnostics).map(Number);
     options.logLevel = options.logLevel.toUpperCase();

src/instances.ts

@@ -89,7 +89,7 @@ export function ensureTypeScriptInstance(
     }
 
     // if allowJs is set then we should accept js(x) files
-    const scriptRegex = configFile.config.compilerOptions.allowJs
+    const scriptRegex = configFile.config.compilerOptions.allowJs && loaderOptions.entryFileIsJs
         ? /\.tsx?$|\.jsx?$/i
         : /\.tsx?$/i;
 

src/interfaces.ts

@@ -155,6 +155,7 @@ export interface LoaderOptions {
     visualStudioErrorFormat: boolean;
     compilerOptions: typescript.CompilerOptions;
     appendTsSuffixTo: RegExp[];
+    entryFileIsJs: boolean;
 }
 
 export interface TSFile {

src/logger.ts

@@ -14,11 +14,11 @@ interface InternalLoggerFunc {
     (whereToLog: any, messages: string[]): void;
 }
 
-const doNothingLogger = (...messages: string[]) => {};
+const doNothingLogger = (..._messages: string[]) => {};
 
 function makeLoggerFunc(loaderOptions: interfaces.LoaderOptions) {
     return loaderOptions.silent 
-        ? (whereToLog: any, messages: string[]) => {}
+        ? (_whereToLog: any, _messages: string[]) => {}
         : (whereToLog: any, messages: string[]) => console.log.apply(whereToLog, messages);
 }
 

src/tsconfig.json

@@ -4,7 +4,7 @@
         "noImplicitReturns": true,
         "noImplicitThis": false,
         "noUnusedLocals": true,
-        "noUnusedParameters": false,
+        "noUnusedParameters": true,
         "suppressImplicitAnyIndexErrors": true,
         "strictNullChecks": false,
         "module": "commonjs",

test/comparison-tests/README.md

@@ -73,3 +73,7 @@ patch0 is applied:
 - test/someFeature/patch0/app.ts - *modified file*
 - test/someFeature/expectedOutput/patch0/bundle.js - *bundle after applying patch*
 - test/someFeature/expectedOutput/patch0/output.txt - *output after applying patch*
+
+## Flaky tests
+
+Some of the tests in the pack are flaky.  For the most part the failures they occasionally experience are not significant.  If you want a test to be allowed to fail without failing the overall build whilst still seeing the output then place a file with the name `_FLAKY_` in the root of that particular test.

test/comparison-tests/allowJs/expectedOutput-2.0/output.transpiled.txt → test/comparison-tests/allowJs-entryFileIsJs/expectedOutput-2.0/output.transpiled.txt

@@ -1,4 +1,4 @@
     Asset     Size  Chunks             Chunk Names
 bundle.js  1.43 kB       0  [emitted]  main
 chunk    {0} bundle.js (main) 38 bytes [rendered]
-    [0] ./.test/allowJs/src/index.js 38 bytes {0} [built]
+    [0] ./.test/allowJs-entryFileIsJs/src/index.js 38 bytes {0} [built]

test/comparison-tests/allowJs/expectedOutput-2.0/output.txt → test/comparison-tests/allowJs-entryFileIsJs/expectedOutput-2.0/output.txt

@@ -1,4 +1,4 @@
     Asset     Size  Chunks             Chunk Names
 bundle.js  1.41 kB       0  [emitted]  main
 chunk    {0} bundle.js (main) 24 bytes [rendered]
-    [0] ./.test/allowJs/src/index.js 24 bytes {0} [built]
+    [0] ./.test/allowJs-entryFileIsJs/src/index.js 24 bytes {0} [built]

test/comparison-tests/allowJs/expectedOutput-2.1/output.transpiled.txt → test/comparison-tests/allowJs-entryFileIsJs/expectedOutput-2.1/output.transpiled.txt

@@ -1,4 +1,4 @@
     Asset     Size  Chunks             Chunk Names
 bundle.js  1.43 kB       0  [emitted]  main
 chunk    {0} bundle.js (main) 38 bytes [rendered]
-    [0] ./.test/allowJs/src/index.js 38 bytes {0} [built]
+    [0] ./.test/allowJs-entryFileIsJs/src/index.js 38 bytes {0} [built]

test/comparison-tests/allowJs/expectedOutput-2.1/output.txt → test/comparison-tests/allowJs-entryFileIsJs/expectedOutput-2.1/output.txt

@@ -1,4 +1,4 @@
     Asset     Size  Chunks             Chunk Names
 bundle.js  1.41 kB       0  [emitted]  main
 chunk    {0} bundle.js (main) 24 bytes [rendered]
-    [0] ./.test/allowJs/src/index.js 24 bytes {0} [built]
+    [0] ./.test/allowJs-entryFileIsJs/src/index.js 24 bytes {0} [built]

test/comparison-tests/allowJs/webpack.config.js → test/comparison-tests/allowJs-entryFileIsJs/webpack.config.js

@@ -13,6 +13,9 @@ module.exports = {
         loader: 'ts-loader'
       }
     ]
+  },
+  ts: {
+    entryFileIsJs: true
   }
 }
 

test/comparison-tests/nodeModulesMeaningfulErrorWhenImportingTs/app.ts

@@ -0,0 +1,3 @@
+import a = require('a');
+
+console.log(a);

test/comparison-tests/nodeModulesMeaningfulErrorWhenImportingTs/expectedOutput-2.0/bundle.js

@@ -0,0 +1,53 @@
+/******/ (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] = {
+/******/ 			exports: {},
+/******/ 			id: moduleId,
+/******/ 			loaded: false
+/******/ 		};
+
+/******/ 		// Execute the module function
+/******/ 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
+
+/******/ 		// Flag the module as loaded
+/******/ 		module.loaded = 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;
+
+/******/ 	// __webpack_public_path__
+/******/ 	__webpack_require__.p = "";
+
+/******/ 	// Load entry module and return exports
+/******/ 	return __webpack_require__(0);
+/******/ })
+/************************************************************************/
+/******/ ([
+/* 0 */
+/***/ function(module, exports, __webpack_require__) {
+
+	"use strict";
+	var a = __webpack_require__(!(function webpackMissingModule() { var e = new Error("Cannot find module \"a\""); e.code = 'MODULE_NOT_FOUND'; throw e; }()));
+	console.log(a);
+
+
+/***/ }
+/******/ ]);

test/comparison-tests/nodeModulesMeaningfulErrorWhenImportingTs/expectedOutput-2.0/bundle.transpiled.js

@@ -0,0 +1,62 @@
+/******/ (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] = {
+/******/ 			exports: {},
+/******/ 			id: moduleId,
+/******/ 			loaded: false
+/******/ 		};
+
+/******/ 		// Execute the module function
+/******/ 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
+
+/******/ 		// Flag the module as loaded
+/******/ 		module.loaded = 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;
+
+/******/ 	// __webpack_public_path__
+/******/ 	__webpack_require__.p = "";
+
+/******/ 	// Load entry module and return exports
+/******/ 	return __webpack_require__(0);
+/******/ })
+/************************************************************************/
+/******/ ([
+/* 0 */
+/***/ function(module, exports, __webpack_require__) {
+
+	"use strict";
+	var a = __webpack_require__(1);
+	console.log(a);
+
+
+/***/ },
+/* 1 */
+/***/ function(module, exports) {
+
+	"use strict";
+	var elephant = "In the room";
+	module.exports = elephant;
+
+
+/***/ }
+/******/ ]);

test/comparison-tests/nodeModulesMeaningfulErrorWhenImportingTs/expectedOutput-2.0/output.transpiled.txt

@@ -0,0 +1,5 @@
+    Asset    Size  Chunks             Chunk Names
+bundle.js  1.6 kB       0  [emitted]  main
+chunk    {0} bundle.js (main) 123 bytes [rendered]
+    [0] ./.test/nodeModulesMeaningfulErrorWhenImportingTs/app.ts 52 bytes {0} [built]
+    [1] ./.test/nodeModulesMeaningfulErrorWhenImportingTs/~/a/index.ts 71 bytes {0} [built]

test/comparison-tests/nodeModulesMeaningfulErrorWhenImportingTs/expectedOutput-2.0/output.txt

@@ -0,0 +1,12 @@
+    Asset    Size  Chunks             Chunk Names
+bundle.js  1.6 kB       0  [emitted]  main
+chunk    {0} bundle.js (main) 52 bytes [rendered]
+    [0] ./.test/nodeModulesMeaningfulErrorWhenImportingTs/app.ts 52 bytes {0} [built] [1 error]
+
+ERROR in ./.test/nodeModulesMeaningfulErrorWhenImportingTs/~/a/index.ts
+Module build failed: Error: Typescript emitted no output for node_modules\a\index.ts.
+You should not need to recompile .ts files in node_modules.
+Please contact the package author to advise them to use --declaration --outDir.
+More https://github.com/Microsoft/TypeScript/issues/12358
+    at Object.loader (dist\index.js:30:15)
+ @ ./.test/nodeModulesMeaningfulErrorWhenImportingTs/app.ts 2:8-20

test/comparison-tests/nodeModulesMeaningfulErrorWhenImportingTs/expectedOutput-2.1/bundle.js

@@ -0,0 +1,53 @@
+/******/ (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] = {
+/******/ 			exports: {},
+/******/ 			id: moduleId,
+/******/ 			loaded: false
+/******/ 		};
+
+/******/ 		// Execute the module function
+/******/ 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
+
+/******/ 		// Flag the module as loaded
+/******/ 		module.loaded = 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;
+
+/******/ 	// __webpack_public_path__
+/******/ 	__webpack_require__.p = "";
+
+/******/ 	// Load entry module and return exports
+/******/ 	return __webpack_require__(0);
+/******/ })
+/************************************************************************/
+/******/ ([
+/* 0 */
+/***/ function(module, exports, __webpack_require__) {
+
+	"use strict";
+	var a = __webpack_require__(!(function webpackMissingModule() { var e = new Error("Cannot find module \"a\""); e.code = 'MODULE_NOT_FOUND'; throw e; }()));
+	console.log(a);
+
+
+/***/ }
+/******/ ]);