Bug with tsconfig paths in #151

[guybedford]

If you create any test.ts in the base of ncc itself when cloned then run:

node ./src/cli.js build test.ts -o tsb

I get the error:

ERROR in ./test.ts
Module build failed (from ./src/loaders/ts-loader.js):
Error: error while parsing tsconfig.json
    at Object.loader (/Users/guybedford/Projects/ncc/node_modules/ts-loader/dist/index.js:19:18)
    at Object.module.exports (/Users/guybedford/Projects/ncc/src/loaders/ts-loader.js:27:17)
    at compiler.run (/Users/guybedford/Projects/ncc/src/index.js:222:23)
    at finalCallback (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:210:39)
    at hooks.done.callAsync.err (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:226:13)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/Hook.js:154:20)
    at onCompiled (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:224:21)
    at hooks.afterCompile.callAsync.err (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:552:14)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/Hook.js:154:20)
    at compilation.seal.err (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:549:30)

If I do create a tsconfig.json file containing:

{
  "compilerOptions": {
    "target": "es2015",
    "moduleResolution": "node"
  }
}

and run the same thing again I get a new error:

Entrypoint main = index.js index.js.map
[0] ./test.ts + 1 modules 162 bytes {0} [built]
    | ./test.ts 115 bytes [built]
    | ./test2.ts 47 bytes [built]

ERROR in /Users/guybedford/Projects/ncc/test/unit/tsconfig-paths/input.ts
[tsl] ERROR in /Users/guybedford/Projects/ncc/test/unit/tsconfig-paths/input.ts(1,20)
      TS2307: Cannot find module '@module'.

ERROR in /Users/guybedford/Projects/ncc/test/unit/tsconfig-paths-conflicting-external/input.ts
[tsl] ERROR in /Users/guybedford/Projects/ncc/test/unit/tsconfig-paths-conflicting-external/input.ts(1,20)
      TS2307: Cannot find module '@module'.
    at compiler.run (/Users/guybedford/Projects/ncc/src/index.js:222:23)
    at finalCallback (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:210:39)
    at hooks.done.callAsync.err (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:226:13)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/Hook.js:154:20)
    at onCompiled (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:224:21)
    at hooks.afterCompile.callAsync.err (/Users/guybedford/Projects/ncc/node_modules/webpack/lib/Compiler.js:552:14)
    at _err0 (eval at create (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:11:1)
    at /Users/guybedford/Projects/ncc/node_modules/ts-loader/dist/after-compile.js:28:9
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/guybedford/Projects/ncc/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:7:1)

where reverting #151 fixes both of the above.

I wasn't able to get this to replicate into the unit tests directory unfortunately.

//cc @odensc

[rauchg]

Will revert #151

[odensc]

I'm getting the first error both with and without #151. Can you provide a repro repo? The error isn't from any code I added either - it's from ts-loader, you need a tsconfig to compile with it.

The second error is due to TypeScript's behavior, it will typecheck every .ts file in the directory by default, and that unit test requires a specific tsconfig file to compile, which is provided in the unit test folder. This error isn't directly caused by my PR besides it adding a TS unit test, it's simply a side effect of having multiple TS projects in the same folder.

What is the real bug here? Is compiling a .ts file in the root of ncc's directory a common use case?

As you can see from the tsconfig docs:

You can fix this by simply limiting the scope of the tsconfig to the proper files:

"files": ["./test.ts"]

[rauchg]

@odensc reverted the revert 😛

[odensc]

Cool. If it's an issue, you could consider putting a tsconfig.json in the root of the repo excluding the test directory:

{
  "exclude": ["./test"]
}

[guybedford]

Ahh yes of course, the first case was my mistake.

As for the second, the fix you offered does help, thanks.

Perhaps it's more about usability then:

1. The error message was not at all clear... that a nested tsconfig and ts file can break a lower one seems quite brittle to begin with. That as a user there was no clear path to know the fix is also unfortunate.
2. Is the above check being done always even if I don't load a ".ts" file but still have a folder with ts files and a tsconfig.json?

[odensc]

1. It is indeed confusing, however these messages come straight from tsc which isn't known for being particularly user-friendly :)
   In that manner I consider it an upstream issue - the only way I can think of to make it nice on our end is detect if there was a tsc error and link to some kind of FAQ page.
2. It will only be done if you build a .ts file, otherwise ts-loader will never be called so it shouldn't check.

[guybedford]

In that manner I consider it an upstream issue - the only way I can think of to make it nice on our end is detect if there was a tsc error and link to some kind of FAQ page.

An upstream issue to track the usability would be really great.

It will only be done if you build a .ts file, otherwise ts-loader will never be called so it shouldn't check.

Thanks for clarifying, that helps a lot to know.

[guybedford]

One thing I missed here was that @module was the actual require in the test case - I thought this was some kind of internal specifier.

Given this, I think the error had all the right information.

In an ideal world I would prefer not to have files that are not explicitly loaded throwing type errors (can we silence these / ignore them?), as that seems unfortunate. But if that is the way it is I can certainly understand that too.