Allow configuration of NodeBuilderFlags.AllowNodeModulesRelativePaths for un-distributed mono repos

[joeljeske]

Search Terms

• AllowNodeModulesRelativePaths
• The inferred type of ... cannot be named without a reference to ...
• Package dependencies and The inferred type of ... cannot be named without a reference to ... #29808
• types in transitive dependencies causing errors after upgrade from 3.1: The inferred type of ... cannot be named without a reference to #30858
• Build better import paths for declaration emit/typeToString from reexports if possible #27340

Suggestion

Allow the optional use of AllowNodeModulesRelativePaths during declaration creation

Use Cases

I understand why @weswigham added #27340 and it is correct for most consumers, especially those that publish packages for consumption on NPM.

However, it seems like for npm style mono-repos where packages are only consumed locally and not published to a registry, it is valid to have seemingly non-portable TS references (lerna, pnpm, rush, yarn workspaces). A relative path to a node_module may still be valid as long as the structure stays consistent, which it does for this style of mono repo with a single node_modules directory.

I am using a mono repo managed by Bazel and rules_nodejs and users are experiencing issues with the creation of declaration files that otherwise would contain relative paths to node_module files. bazel-contrib/rules_nodejs#1013

(The conversation in #30858 was helpful, however, it seemed to still be focusing on resolving portable types, which is only necessary when distributing the declaration files. If they are consumed locally, portability is not a concern.

Examples

I'm not sure the best way (or standard way) for these flags to be set, however, I imagine an entry in the TSConfig Compiler Options would make the most sense:

interface CompilerOptions {
  ...
+  allowUnportableDeclarationImports: true
} 

Currently I am patching Typescript to toggle on this behavior and it appears it is working just as expected.

typescript+3.7.3.patch.txt

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

[RyanCavanaugh]

@weswigham I'm interested to hear your thoughts on this

[weswigham]

@joeljeske you only want this for incremental builds, right? Since if you trigger this error there's no way you're shipping the resulting .d.ts file as a redistributable.

[joeljeske]

Well no actually. I am using bazel with rules nodejs, but let me describe how it works. The build consists of many packages that link to each other via tsconfig path mappings (not via symlinking into node modules). Each package is built independently and only sees the type declarations of other local dependencies. The local dependencies all share a common top level node modules directory. In this way, if a package generates a declaration file with a relative import path into the node modules dir then downstream local packages that depend on that declaration file would still be able to resolve that import.

None of the declarations files are ever used outside the mono repo

Does that make much sense?

[weswigham]

Sure; but what are the declaration files being built for? It's certainly not for publishing for consumption if you're OK with paths like this (since there's absolutely no way these paths could ever be shipped outside a specialized environment), so the only other use would be incremental style (similar to --incremental or --build mode) builds. As far as I know, bazel's default mode of operation, too, is incremental in nature.

[joeljeske]

The declaration files are used for type checking against other local packages. They never leave the repo.

Example: local module A depends on local module B. The generated declaration files of B are used during the compilation of A.

Yes bazel is incremental, but not in the sense of using —incremental as far as I know. It is incremental in the sense that module A will only be recompiled if the declaration files (public api) of module B changes.

My mono repo has many small Packages in typescript that are used in a variety of places. The end product of the mono repo is a set of compiled JS applications ready for runtime. the d.ts files are only used in the intermediary build process as described.

[weswigham]

The declaration files are used for type checking against other local packages. They never leave the repo.

Right - pretty much the definition of an incremental build. I can see removing the error when incremental is set but declaration is not expressly set, however directly exposing an internal like this is not something I'd be comfortable with.

[joeljeske]

Yea I guess i was thinking about an incremental build from the standpoint of a single package. In my case each package is fully rebuilt at a time, not using the incremental TS option. But looking from the standpoint of the entire repo, yes it is incremental; some packages will have TSC invoked on them and some will not.

Regardless of the terminology, I can say that I am not using TS incremental option and I am explicitly requesting the declaration files. I do respect concern of exposing implementation details, however, I feel like I have a valid use case here that does not fit into your limited exception:

when incremental is set but declaration is not expressly set

[flolu]

I definitely agree, that @joeljeske has a valid use case here.

Especially, I do not think it would hurt anybody if it is disabled by default and can be enabled in the compiler options when needed.

[joeljeske]

@weswigham @RyanCavanaugh, do y'all have any additional thoughts regarding my use case? Would any supporting examples or additional use case explanations be of help?