ts-loader can't find typings for npm TS module when allowJS is true

[bsouthga]

Minimal repro repository: https://github.com/bsouthga/ts-loader-types-error-example

First off, thanks for all the great work you guys do on ts-loader, its an important part of our build stack!

Issue:

Versions:

• webpack@3.3.0
• typescript@2.4.1
• ts-loader@2.3.0

Given a JS entry file:

// `index.js`
import './imported'

And an imported typescript file (importing this library):

// `imported.ts`
import { EventRank } from 'event-rank';
console.log(EventRank);

And a webpack config as follows:

module.exports = {
  entry: './src/entry.js',
  output: {
    filename: 'bundle.js'
  },
  resolve: {
    // Add `.ts` and `.tsx` as a resolvable extension.
    extensions: ['.ts', '.tsx', '.js'] // note if using webpack 1 you'd also need a '' in the array as well
  },
  module: {
    loaders: [ // loaders will work with webpack 1 or 2; but will be renamed "rules" in future
      // all files with a `.ts` or `.tsx` extension will be handled by `ts-loader`
      { test: /\.tsx?$/, loader: 'ts-loader', options: {
        entryFileIsJs: true
      } }
    ]
  }
}

The project will compile fine under tsc, but webpack returns the following error:

ts-loader: Using typescript@2.4.1 and ~/projects/ts-loader-error/tsconfig.json
Hash: 836c8ecb865d0c8010fe
Version: webpack 3.3.0
Time: 859ms
    Asset     Size  Chunks             Chunk Names
bundle.js  42.4 kB       0  [emitted]  main
   [1] ./src/entry.js 20 bytes {0} [built]
   [2] ./src/imported.ts 64 bytes {0} [built] [1 error]
    + 3 hidden modules

ERROR in ./src/imported.ts
(1,10): error TS2305: Module '"~/projects/ts-loader-error/node_modules/event-rank/dist/src/index"' hasno exported member 'EventRank'.

[johnnyreilly]

Hey @bsouthga,

I've had a quick look but nothing leaped out at me. I'm afraid I don't have time to look into this right now but I'm happy to support you as you do.

There's very little magic around the entryFileIsJs flag; it literally amounts to this regex change:

https://github.com/TypeStrong/ts-loader/blob/master/src/instances.ts#L96

I'd advise cloning a copy of ts-loader, npm linking it and then tweaking it to log out extra info. If you're feeling bold 😉

[bsouthga]

will do, thanks for the quick feedback!

[bsouthga]

So I dug into this a little bit, and it looks like the issue is here:

ts-loader/src/servicesHost.ts

Lines 146 to 150 in 80e0349

	if (resolutionResult) {
	if (resolutionResult.resolvedFileName === tsResolutionResult.resolvedFileName) {
	resolutionResult.isExternalLibraryImport = tsResolutionResult.isExternalLibraryImport;
	}
	} else {

When allowJS is true, originalFileName (index.js) matches scriptRegex which means that resolutionResult exists. However, typescript resolves the file to be the bundled type declaration file (index.d.ts) and correctly identifies it as an external module. So...

resolutionResult.resolvedFileName !== tsResolutionResult.resolvedFileName

Meaning the returned resolution result has:

resolutionResult.isExternalLibraryImport === undefined

As a fix, I added a helper function and changed the line above as follows:

function isBundledTypeDeclaration(resolvedFileName: string, tsResolvedFileName: string) {
    return resolvedFileName.replace(/\.js$/, '.d.ts') === tsResolvedFileName;
}

// lines omitted...
        if (resolutionResult) {
            if (resolutionResult.resolvedFileName === tsResolutionResult.resolvedFileName ||
                isBundledTypeDeclaration(resolutionResult.resolvedFileName, tsResolutionResult.resolvedFileName)) {
                resolutionResult.isExternalLibraryImport = tsResolutionResult.isExternalLibraryImport;
            }
        } else {
            resolutionResult = tsResolutionResult;
        }
    }
    return resolutionResult;
}

It seems like a bit of a hack, as its possible that the bundled declaration files are not named the same or in the same directory as the js. Any alternate thoughts?

[johnnyreilly]

First up, well done for identifying the problem!

It seems like a bit of a hack, as its possible that the bundled declaration files are not named the same or in the same directory as the js. Any alternate thoughts?

Yeah - it's a massive hack 😉

Couple of ideas - can you give me some example values for the 2 values in the expression below?:

            if (resolutionResult.resolvedFileName === tsResolutionResult.resolvedFileName)

Are we dealing with a filename or a full path? What I'm wondering is, are there other rules we can use here instead of the one you proposed. Idea:

Assuming we have full paths then we can presumably check for the presence of node_modules and .d.ts. So something along the lines of:

        if (resolutionResult) {
            if (resolutionResult.resolvedFileName === tsResolutionResult.resolvedFileName ||
                // not the real regex but you get the gist - cannot do regex before bed
                /node_modules\/*.d.ts/.test(tsResolutionResult.resolvedFileName)
                ) {
                resolutionResult.isExternalLibraryImport = tsResolutionResult.isExternalLibraryImport;
            }
        } else {
            resolutionResult = tsResolutionResult;
        }
    }
    return resolutionResult;

Would that work?

[bsouthga]

yeah, switching the conditional to

        if (resolutionResult) {
            if (resolutionResult.resolvedFileName === tsResolutionResult.resolvedFileName ||
                /node_modules\/.*\.d\.ts/.test(tsResolutionResult.resolvedFileName)) {
                resolutionResult.isExternalLibraryImport = tsResolutionResult.isExternalLibraryImport;
            }
        } else {
            resolutionResult = tsResolutionResult;
        }

solves the issue as well, for reference logging

        console.log({
            loader: resolutionResult.resolvedFileName,
            ts: tsResolutionResult.resolvedFileName
        });

produces

{ loader: '/Users/bensouthgate/projects/ts-loader-error/src/imported.ts',
  ts: '/Users/bensouthgate/projects/ts-loader-error/src/imported.ts' }
{ loader: '/Users/bensouthgate/projects/ts-loader-error/node_modules/event-rank/dist/src/index.js',
  ts: '/Users/bensouthgate/projects/ts-loader-error/node_modules/event-rank/dist/src/index.d.ts' }

[bsouthga]

seems to cause issues with the tests though, I'll keep poking around

[johnnyreilly]

Cool - let me know what your findings are. Also some of the tests are supersensitive so it could be worth checking the nature of the errors. (comparison test pack in particular) Test "failures" aren't always failures - if you follow me....

[johnnyreilly]

Oh and feel free to submit a PR with your change. That will allow me to see the nature of the failing tests and I can probably advise on whether it's a meaningful failure.

We're trying to move (where possible) to using the execution test pack as it's less flaky than the comparison test pack. (There's some test scenarios that require the comparison test pack though)

[johnnyreilly]

Hey @bsouthga, just wondering how your investigations went?

[johnnyreilly]

I've tried to add a speculative test for this here: #590

It's based on your repro - might need some tweaking to get working. It's more complicated than I'd like as it depends on babel due to EventRank using ES2015 features. Longer term it might made for a clearer test to base our execution test on this one: https://github.com/TypeStrong/ts-loader/tree/master/test/execution-tests/1.8.2_babel-allowSyntheticDefaultImports (I'm pretty sure babel can be stripped out here as well - I don't think it's required)

Hopefully this can get the ball rolling though. The test fails - hopefully for the reason we expect. We now just need to apply the speculative fix and see if that resolves matters.

[johnnyreilly]

Closed with #590