Merge pull request #658 from johnnyreilly/master

Fix allowJs @types error - closes #657 #655

.npmignore

@@ -1,5 +1,6 @@
 *.ts
 .vscode
 .test
+examples
 test
 src

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## v3.0.3
+
+- [Fix allowJs @types resolution error](https://github.com/TypeStrong/ts-loader/pull/658) (#657, #655) - thanks @johnnyreilly and @roddypratt + @ldrick for providing minimal repro repos which allowed me to fix this long standing bug!
+
+This fix resolves the issue for TypeScript 2.4+ (which is likely 95% of users).  For those people stuck on 2.4 and impacted by this issue, you should be able to workaround this by setting `entryFileCannotBeJs: true` in your ts-loader options.  This option should be considered deprecated as of this release.  The option will likely disappear with the next major version of ts-loader which will drop support for TypeScript 2.3 and below, thus removing the need for this option.
+
 ## v3.0.0
 
 All changes were made with this [PR](https://github.com/TypeStrong/ts-loader/pull/643) - thanks @johnnyreilly

README.md

@@ -296,9 +296,11 @@ 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.
 
-#### entryFileCannotBeJs *(boolean) (default=false)*
+#### entryFileCannotBeJs *(boolean) (default=false) DEPRECATED*
 
-If the `allowJs` compiler option is `true` then it's possible for your entry files to be JS. Since people have reported occasional problems with this the `entryFileCannotBeJs` setting exists to disable this functionality (if set then your entry file cannot be JS).  Please note that this is rather unusual and will generally not be necessary when using `allowJs`.  This option may be removed in a future version of ts-loader if it appears to be unused (likely).
+If the `allowJs` compiler option is `true` then it's possible for your entry files to be JS. There is a [known issue using ts-loader with TypeScript 2.3 and below](https://github.com/TypeStrong/ts-loader/issues/655).  This option exists to work around that issue if you are using ts-loader with TypeScript 2.3 or below.
+
+This option will be removed in a future version of ts-loader.
 
 #### appendTsSuffixTo *(RegExp[]) (default=[])*
 #### appendTsxSuffixTo *(RegExp[]) (default=[])*

package.json

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

src/compilerSetup.ts

@@ -44,9 +44,9 @@ export function getCompilerOptions(
     configParseResult: typescript.ParsedCommandLine
 ) {
     const compilerOptions = Object.assign({}, configParseResult.options, {
-        skipDefaultLibCheck: true,
+        skipLibCheck: true,
         suppressOutputPathCheck: true, // This is why: https://github.com/Microsoft/TypeScript/issues/7363
-    });
+    } as typescript.CompilerOptions);
 
     // if `module` is not specified and not using ES6+ target, default to CJS module output
     if ((compilerOptions.module === undefined) && 

src/index.ts

@@ -110,7 +110,7 @@ function getLoaderOptions(loader: Webpack) {
 }
 
 type ValidLoaderOptions = keyof LoaderOptions;
-const validLoaderOptions: ValidLoaderOptions[] = ['silent', 'logLevel', 'logInfoToStdOut', 'instance', 'compiler', 'configFile', 'transpileOnly', 'ignoreDiagnostics', 'errorFormatter', 'colors', 'compilerOptions', 'appendTsSuffixTo', 'appendTsxSuffixTo', 'entryFileCannotBeJs', 'happyPackMode', 'getCustomTransformers'];
+const validLoaderOptions: ValidLoaderOptions[] = ['silent', 'logLevel', 'logInfoToStdOut', 'instance', 'compiler', 'configFile', 'transpileOnly', 'ignoreDiagnostics', 'errorFormatter', 'colors', 'compilerOptions', 'appendTsSuffixTo', 'appendTsxSuffixTo', 'entryFileCannotBeJs' /* DEPRECATED */, 'happyPackMode', 'getCustomTransformers'];
 
 /**
  * Validate the supplied loader options.

src/interfaces.ts

@@ -211,7 +211,7 @@ export interface ResolveSync {
 
 export interface ModuleResolutionHost {
     fileExists(fileName: string): boolean;
-    readFile(fileName: string): string;
+    readFile(fileName: string, encoding?: string | undefined): string | undefined;
 }
 
 export interface TSInstance {
@@ -273,6 +273,7 @@ export interface LoaderOptions {
     compilerOptions: typescript.CompilerOptions;
     appendTsSuffixTo: RegExp[];
     appendTsxSuffixTo: RegExp[];
+    /** DEPRECATED */
     entryFileCannotBeJs: boolean;
     happyPackMode: boolean;
     getCustomTransformers?(): typescript.CustomTransformers | undefined;

src/servicesHost.ts

@@ -1,11 +1,12 @@
 import * as typescript from 'typescript';
 import * as path from 'path';
+import * as semver from 'semver';
 
 import * as constants from './constants';
 import * as logger from './logger';
 import { makeResolver } from './resolver';
 import { appendSuffixesIfMatch, readFile } from './utils';
-import { 
+import {
     ModuleResolutionHost,
     ResolvedModule,
     ResolveSync,
@@ -28,25 +29,45 @@ export function makeServicesHost(
 
     const newLine =
         compilerOptions.newLine === constants.CarriageReturnLineFeedCode ? constants.CarriageReturnLineFeed :
-        compilerOptions.newLine === constants.LineFeedCode ? constants.LineFeed :
-        constants.EOL;
+            compilerOptions.newLine === constants.LineFeedCode ? constants.LineFeed :
+                constants.EOL;
 
     // make a (sync) resolver that follows webpack's rules
     const resolveSync = makeResolver(loader.options);
 
+    const readFileWithFallback = compiler.sys === undefined || compiler.sys.readFile === undefined
+        ? readFile
+        : (path: string, encoding?: string | undefined): string | undefined => compiler.sys.readFile(path, encoding) || readFile(path, encoding);
+
+    const fileExists = compiler.sys === undefined || compiler.sys.fileExists === undefined
+        ? (path: string) => readFile(path) !== undefined
+        : (path: string) => compiler.sys.fileExists(path) || readFile(path) !== undefined;
+
     const moduleResolutionHost: ModuleResolutionHost = {
-        fileExists: (fileName: string) => readFile(fileName) !== undefined,
-        readFile: (fileName: string) => readFile(fileName) || '',
+        fileExists,
+        readFile: readFileWithFallback
     };
 
-    return {
+    // loader.context seems to work fine on Linux / Mac regardless causes problems for @types resolution on Windows for TypeScript < 2.3
+    const getCurrentDirectory = (compiler!.version && semver.gte(compiler!.version, '2.3.0'))
+        ? () => loader.context
+        : () => process.cwd();
+
+    const resolutionStrategy = (compiler!.version && semver.gte(compiler!.version, '2.4.0'))
+        ? resolutionStrategyTS24AndAbove
+        : resolutionStrategyTS23AndBelow;
+
+    const servicesHost: typescript.LanguageServiceHost = {
         getProjectVersion: () => `${instance.version}`,
+
         getScriptFileNames: () => Object.keys(files).filter(filePath => filePath.match(scriptRegex)),
+
         getScriptVersion: (fileName: string) => {
             fileName = path.normalize(fileName);
             const file = files[fileName];
             return file === undefined ? '' : file.version.toString();
         },
+
         getScriptSnapshot: (fileName: string) => {
             // This is called any time TypeScript needs a file's text
             // We either load from memory or from disk
@@ -66,30 +87,44 @@ export function makeServicesHost(
          * getDirectories is also required for full import and type reference completions.
          * Without it defined, certain completions will not be provided
          */
-        getDirectories: compiler.sys ? (<any> compiler.sys).getDirectories : undefined,
+        getDirectories: compiler.sys ? compiler.sys.getDirectories : undefined,
 
         /**
          * For @types expansion, these two functions are needed.
          */
-        directoryExists: compiler.sys ? (<any> compiler.sys).directoryExists : undefined,
+        directoryExists: compiler.sys ? compiler.sys.directoryExists : undefined,
+
+        useCaseSensitiveFileNames: compiler.sys
+            ? () => compiler.sys.useCaseSensitiveFileNames
+            : undefined,
 
         // The following three methods are necessary for @types resolution from TS 2.4.1 onwards see: https://github.com/Microsoft/TypeScript/issues/16772
-        fileExists: compiler.sys ? (<any> compiler.sys).fileExists : undefined,
-        readFile: compiler.sys ? (<any> compiler.sys).readFile : undefined,
-        readDirectory: compiler.sys ? (<any> compiler.sys).readDirectory : undefined,
+        fileExists: compiler.sys ? compiler.sys.fileExists : undefined,
+        readFile: compiler.sys ? compiler.sys.readFile : undefined,
+        readDirectory: compiler.sys ? compiler.sys.readDirectory : undefined,
 
-        getCurrentDirectory: () => process.cwd(),
+        getCurrentDirectory,
 
         getCompilationSettings: () => compilerOptions,
         getDefaultLibFileName: (options: typescript.CompilerOptions) => compiler.getDefaultLibFilePath(options),
         getNewLine: () => newLine,
         log: log.log,
+
+        /* Unclear if this is useful
+        resolveTypeReferenceDirectives: (typeDirectiveNames: string[], containingFile: string) =>
+            typeDirectiveNames.map(directive =>
+                compiler.resolveTypeReferenceDirective(directive, containingFile, compilerOptions, moduleResolutionHost).resolvedTypeReferenceDirective),
+        */
+
         resolveModuleNames: (moduleNames: string[], containingFile: string) =>
             resolveModuleNames(
                 resolveSync, moduleResolutionHost, appendTsSuffixTo, appendTsxSuffixTo, scriptRegex, instance,
-                moduleNames, containingFile),
+                moduleNames, containingFile, resolutionStrategy),
+
         getCustomTransformers: () => instance.transformers
     };
+
+    return servicesHost;
 }
 
 function resolveModuleNames(
@@ -100,12 +135,12 @@ function resolveModuleNames(
     scriptRegex: RegExp,
     instance: TSInstance,
     moduleNames: string[],
-    containingFile: string
+    containingFile: string,
+    resolutionStrategy: ResolutionStrategy
 ) {
     const resolvedModules = moduleNames.map(moduleName =>
         resolveModuleName(resolveSync, moduleResolutionHost, appendTsSuffixTo, appendTsxSuffixTo, scriptRegex, instance,
-            moduleName, containingFile)
-    );
+            moduleName, containingFile, resolutionStrategy));
 
     populateDependencyGraphs(resolvedModules, instance, containingFile);
 
@@ -116,10 +151,12 @@ function isJsImplementationOfTypings(
     resolvedModule: ResolvedModule,
     tsResolution: ResolvedModule
 ) {
-    return resolvedModule.resolvedFileName.endsWith('js') && 
-            /node_modules(\\|\/).*\.d\.ts$/.test(tsResolution.resolvedFileName);
+    return resolvedModule.resolvedFileName.endsWith('js') &&
+        /node_modules(\\|\/).*\.d\.ts$/.test(tsResolution.resolvedFileName);
 }
 
+type ResolutionStrategy = (resolutionResult: ResolvedModule | undefined, tsResolutionResult: ResolvedModule) => ResolvedModule;
+
 function resolveModuleName(
     resolveSync: ResolveSync,
     moduleResolutionHost: ModuleResolutionHost,
@@ -129,7 +166,9 @@ function resolveModuleName(
     instance: TSInstance,
 
     moduleName: string,
-    containingFile: string
+    containingFile: string,
+
+    resolutionStrategy: ResolutionStrategy
 ) {
     const { compiler, compilerOptions } = instance;
 
@@ -159,25 +198,40 @@ function resolveModuleName(
             resolvedFileName,
             isExternalLibraryImport: tsResolution.resolvedModule.isExternalLibraryImport
         };
-        if (resolutionResult!) {
-            if (resolutionResult!.resolvedFileName === tsResolutionResult.resolvedFileName ||
-                isJsImplementationOfTypings(resolutionResult!, tsResolutionResult)) {
-                resolutionResult!.isExternalLibraryImport = tsResolutionResult.isExternalLibraryImport;
-            }
-        } else {
-            resolutionResult = tsResolutionResult;
+
+        return resolutionStrategy(resolutionResult!, tsResolutionResult);
+    }
+    return resolutionResult!;
+}
+
+function resolutionStrategyTS23AndBelow(resolutionResult: ResolvedModule | undefined, tsResolutionResult: ResolvedModule): ResolvedModule {
+    if (resolutionResult! !== undefined) {
+        if (resolutionResult!.resolvedFileName === tsResolutionResult.resolvedFileName ||
+            isJsImplementationOfTypings(resolutionResult!, tsResolutionResult)) {
+            resolutionResult!.isExternalLibraryImport = tsResolutionResult.isExternalLibraryImport;
         }
+    } else {
+        return tsResolutionResult;
     }
     return resolutionResult!;
 }
 
+function resolutionStrategyTS24AndAbove(resolutionResult: ResolvedModule | undefined, tsResolutionResult: ResolvedModule): ResolvedModule {
+    return (resolutionResult! === undefined ||
+        resolutionResult!.resolvedFileName === tsResolutionResult.resolvedFileName ||
+        isJsImplementationOfTypings(resolutionResult!, tsResolutionResult)
+    )
+        ? tsResolutionResult
+        : resolutionResult!;
+}
+
 function populateDependencyGraphs(
     resolvedModules: ResolvedModule[],
     instance: TSInstance,
     containingFile: string
 ) {
     resolvedModules = resolvedModules
-        .filter(m => m !== null && m !== undefined);
+        .filter(mod => mod !== null && mod !== undefined);
 
     instance.dependencyGraph[path.normalize(containingFile)] = resolvedModules;
 

src/utils.ts

@@ -84,10 +84,10 @@ export function formatErrors(
         : [];
 }
 
-export function readFile(fileName: string) {
+export function readFile(fileName: string, encoding: string | undefined = 'utf8') {
     fileName = path.normalize(fileName);
     try {
-        return fs.readFileSync(fileName, 'utf8');
+        return fs.readFileSync(fileName, encoding);
     } catch (e) {
         return undefined;
     }

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

@@ -2,34 +2,6 @@
 bundle.js  2.49 kB       0  [emitted]  main
    [0] ./.test/nolib/app.ts 16 bytes {0} [built] [1 error]
 
-ERROR in node_modules\@types\semver\index.d.ts
-[tsl] ERROR in node_modules\@types\semver\index.d.ts(100,28)
-      TS2304: Cannot find name 'Array'.
-
-ERROR in node_modules\@types\semver\index.d.ts
-[tsl] ERROR in node_modules\@types\semver\index.d.ts(100,70)
-      TS2304: Cannot find name 'Array'.
-
-ERROR in node_modules\@types\semver\index.d.ts
-[tsl] ERROR in node_modules\@types\semver\index.d.ts(104,29)
-      TS2304: Cannot find name 'Array'.
-
-ERROR in node_modules\@types\semver\index.d.ts
-[tsl] ERROR in node_modules\@types\semver\index.d.ts(104,71)
-      TS2304: Cannot find name 'Array'.
-
-ERROR in node_modules\@types\semver\index.d.ts
-[tsl] ERROR in node_modules\@types\semver\index.d.ts(123,41)
-      TS2304: Cannot find name 'Array'.
-
-ERROR in node_modules\@types\semver\index.d.ts
-[tsl] ERROR in node_modules\@types\semver\index.d.ts(127,41)
-      TS2304: Cannot find name 'Array'.
-
-ERROR in node_modules\@types\chalk\index.d.ts
-[tsl] ERROR in node_modules\@types\chalk\index.d.ts(18,10)
-      TS2370: A rest parameter must be of an array type.
-
 ERROR in ./.test/nolib/app.ts
 [tsl] ERROR in app.ts(1,1)
       TS2304: Cannot find name 'parseInt'.

test/execution-tests/2.4.1_nodeResolutionAllowJs/karma.conf.js

@@ -0,0 +1,46 @@
+/* eslint-disable no-var, strict */
+'use strict';
+var path = require('path');
+var webpack = require('webpack');
+var webpackConfig = require('./webpack.config.js');
+var reporterOptions = require('../../reporterOptions');
+
+module.exports = function(config) {
+  config.set({
+    browsers: [ 'ChromeHeadless' ],
+
+    files: [
+      // This loads all the tests
+      'main.js'
+    ],
+
+    port: 9876,
+
+    frameworks: [ 'jasmine' ],
+
+    logLevel: config.LOG_INFO, //config.LOG_DEBUG
+
+    preprocessors: {
+      'main.js': [ 'webpack', 'sourcemap' ]
+    },
+
+    webpack: {
+      devtool: 'inline-source-map',
+      module: webpackConfig.module,
+      resolve: webpackConfig.resolve,
+
+      // for test harness purposes only, you would not need this in a normal project
+      resolveLoader: webpackConfig.resolveLoader
+    },
+
+    webpackMiddleware: {
+      quiet: true,
+      stats: {
+        colors: true
+      }
+    },
+
+    // reporter options
+    mochaReporter: reporterOptions
+  });
+};

test/execution-tests/2.4.1_nodeResolutionAllowJs/main.js

@@ -0,0 +1,2 @@
+const testsContext = require.context('./', true, /\.tests\.ts(x?)$/);
+testsContext.keys().forEach(testsContext);

test/execution-tests/2.4.1_nodeResolutionAllowJs/package.json

@@ -0,0 +1,11 @@
+{
+  "name": "import-code-splitting",
+  "version": "1.0.0",
+  "main": "index.js",
+  "devDependencies": {
+    "@types/jasmine": "^2.5.35"
+  },
+  "dependencies": {
+    "date-fns": "2.0.0-alpha.1"    
+  }
+}