TSC fails to emit required files when run below node_modules

[yofreke]

TypeScript Version: 3.2.0-dev.20181107

Search Terms: node_modules

Code

/tmp/node_modules/tsc-test/tsconfig.json

{
 "compilerOptions": {
 "outDir": "dist"
 },
 "files": [
 "src/index.ts"
 ]
}

/tmp/node_modules/tsc-test/src/index.ts

import { A } from './A';

export const main = () => {
 console.log('A=', A);
};

/tmp/node_modules/tsc-test/src/A.ts

export const A = 'A';

Expected behavior:

Output files:

• /tmp/node_modules/tsc-test/dist/index.js
• /tmp/node_modules/tsc-test/dist/A.js

Actual behavior:

Output files:

/tmp/node_modules/tsc-test/dist/index.js

"use strict";
exports.__esModule = true;
var A_1 = require("./A");
exports.main = function () {
 console.log('A=', A_1.A);
};

No A.js is output

Notes:

I wanted to build a typescript project as part of an npm postinstall script, to enable installation from npm or from git. In the case of installation from npmjs, dist/ will already exist. In the case of installation from git, dist/ will be missing and build will be run.

After setting this up, I noticed that only files listed in my tsconfig files list were being emitted when installing from git. I looked around for tsconfig or tsc options that may change this behavior, but realized it probably had to do with 'node_modules' being in the path.

I debugged the issue by modifying the built tsc.js. In my case, the problematic code is in nodeModuleNameResolverWorker/tryResolve, specifically here:

const { path: candidate, parts } = normalizePathAndParts(combinePaths(containingDirectory, moduleName));
const resolved = nodeLoadModuleByRelativeName(extensions, candidate, /*onlyRecordFailures*/ false, state, /*considerPackageJson*/ true);
// Treat explicit "node_modules" import as an external library import.
return resolved && toSearchResult({ resolved, isExternalLibraryImport: contains(parts, "node_modules") });

The call to contains(parts, "node_modules") should likely consider the project root directory, and ignore "node_modules" in the path parts above the root.

[sheetalkamat]

This is working as intended since we want to not touch anything in node_modules because of build.

[weswigham]

Sheetal Nandi (@sheetalkamat) There are valid reasons to have a project root within a node_modules folder (hoisting, for example). Maybe we should consider adjusting it? The suggested fix seems reasonable - ignoring node_modules if it occurs in the project root path.

[sheetalkamat]

I don't think if it occurs under project root is good enough.. Eg if the project root file references another module from same node_modules but not in projectRoot it needs to skip emitting that and that just makes module resolution depend on too many things then (outside of its resolution

[weswigham]

Right, what I'm getting at is that if the project root is /a/b/node_modules/c/d, anything beneath /a/b/node_modules/c/d should be built. /a/b/node_modules/c shouldn't be, /a/b/node_modules/c/d/node_modules/e shouldn't be.

[weswigham]

So, if you path begins with the project root, you only check the components after the project root ends for node_modules.

[yofreke]

This is working as intended

I hope we can find a way to allow tsc to produce functional builds under these circumstances. As of now tsc produces a non-functional build, which does not seem like intended/optimal behavior.

Eg if the project root file references another module from same node_modules but not in projectRoot

If /tmp/node_modules/tsc-test/src/index.ts references /tmp/node_modules/project2/src/index.ts, the build in tsc-test should exclude /tmp/node_modules/project2*, since this is not part of the project root.

It seems like the ideal solution is to skip emit under two conditions:

• When the built file is not part of project root
• When the built file is part of the project root, and under a node_modules directory

[yofreke]

Hoping to get further guidance from the TS team after "working as intended", any updates on this?

[DanielRosenwasser]

Since Wesley Wigham (@weswigham) seems to have some thoughts around this, we can discuss this in a backlog slog in the future

[cspotcode]

Someone reported this to ts-node. A quirk in our case: we pass empty files and includes arrays to the compiler to prevent it from eagerly pulling all sources into the compiler. Thus, the target of every import statement is unknown to the compiler, and moduleResolution sets isExternalLibraryImport = true

I'm planning to implement the behavior described in #28432 (comment)

Is type-checking affected by isExternalLibraryImport? In other words, if ts-node takes the nuclear option and always forces that flag to be false, will it break typechecking? ts-node has its own --ignore pattern that determines which files are compiled and which are skipped, so it's possible that ts-node's --ignore filtering won't always match TypeScript's isExternalLibraryImport logic. The nuclear option avoids that potential mis-match by effectively disabling TypeScript's filtering, deferring entirely to ts-node's filtering.

EDIT: rephrased for clarity

[rDr4g0n]

I spent a few days troubleshooting before learning of this behavior. My use case is pretty straightforward.

I have a git repo, Repo A, with an npm postinstall script which runs ng-packagr (which indirectly calls tsc) to build itself as an angular library.

I npm install RepoA as a dependency of a separate repo, Repo B, but the Repo A's postinstall build fails.

After much investigation, it turns out that tsc is refusing to include almost all of Repo A's ts files because they are in Repo B's node_modules directory.

I believe running tsc and other tools in the npm postinstall lifecycle hook in order to build a library for consumption is a reasonable use case. It is unfortunate that it is not supported by tsc. Can it at least be configurable in some way? Is there a workaround?

[edit] Reviewing the code, it looks like isExternalLibraryImport is set to true if the string "node_modules" appears anywhere in the path, but I think its safe to say that in the case I describe above, those ts files are not external library imports. They are external to Repo B, but they are certainly local to Repo A, and Repo A is the one doing the build work at the moment.

In this scenario, the check to see what is external is too simplistic to be correct.