With project references, some cross-package imports of inferred types are emitted with relative paths

[ecraig12345]

TypeScript Version: 3.9.5 (also repros with nightly)

Search Terms: project references, declaration files, tsconfig paths, inferred types

Code

The repro requires a monorepo with project references plus paths in the tsconfig. Full repro with the problematic compiled output checked in here: https://github.com/ecraig12345/learn-a/tree/inferred-relative-imports (it's fairly minimal but may still include a few settings which turn out to be tangential to the issue, sorry)

Here's a summary...

@fluentui/pkg2 depends on @fluentui/pkg1, and both live in the same monorepo under a packages folder.

pkg2's tsconfig has a project reference pointing to pkg1, and both build with composite: true.

They inherit from a shared tsconfig which has a path config like this (to allow imports directly from the root of each package):

"baseUrl": ".",
"paths": {
  "@fluentui/*": ["packages/*/src"]
}

The problem comes when you have a file in one package (pkg2/src/index.ts in the repro) which has an implicit/inferred reference to a type from another local project-referenced package.

import { IThings } from '@fluentui/pkg1';

export function fn4() { // inferred return type: IThings from pkg1
  const a: IThings = { thing1: { a: 'b' } };
  return a.thing1;
}

This inferred type need not be stated locally, but it will show up in the declaration file as an inline import().

Expected behavior:

Compile each package with tsc (or in the repro, run yarn build from the root).

pkg2's index.d.ts looks like this:

export declare function fn4(): import("@fluentui/pkg1").IThing;

Actual behavior:

pkg2's index.d.ts looks like this, which will break consumers:

export declare function fn4(): import("../../pkg1/src").IThing;

pkg2/src/index.ts output also adds an src folder under lib, which I wouldn't expect (pkg2/lib/src/index.{js,d.ts}), but this is probably unrelated.

Variants:

• Same behavior if -b is added to the build command.
• Same behavior if using rootDir instead of include, just different output directory structure.
• Same behavior if the config inheritance is removed and paths config is moved to each project.
• Without project references, you end up with a directory structure like this, which I also don't understand, but is not the same issue:

pkg2/
  lib/
    pkg1/src/<files>
    pkg2/src/<files>

Playground Link: can't be shown on playground

Related Issues: didn't see anything else quite like this

[robpalme]

Hello @ecraig12345, this is a duplicate of #38111

I wrote up our solution there. The summary is that when wiring up dependencies, "paths" currently won't give you what you want. Ambient module declarations are the only way to avoid the issue you describe.

It would be awesome if "paths" had the same super-power as ambient module declarations, because we made the same incorrect assumption that "paths" would give us bare-specifier linkage for free.

[daniele-orlando]

I can reproduce this bug after upgrading TypeScript from version 3.7 to 3.9.7.

I have a monorepo with:
packages/std-react using packages/std-web which is symlinked inside packages/std-react/node_modules.

Building packages/std-react throws this error

../std-web/router.ts:77:17 - error TS2742: The inferred type of 'createHistoryRouter' cannot be named without a reference to '../std-react/node_modules/@types/react'. This is likely not portable. A type annotation is necessary.

but if I copy and paste packages/std-web inside packages/std-react/node_modules, the error goes away.

[sheetalkamat]

Inferred module specifiers in d.ts file are suppose to be relative so this current behavior is correct but we could prefer the existing import in the file if it refers to same file and i have a PR #42224 to try that out and see how it works out

[ecraig12345]

Inferred module specifiers in d.ts file are suppose to be relative so this current behavior is correct

How is this correct if the module is in a different project? Relative inferred module specifiers are a huge problem for consumers of our .d.ts files because it's possible/likely that they just flat-out won't work if the folder structure in the consumer's node_modules is different than our monorepo's internal folder structure. Also, there are other existing cases where the inferred specifier is NOT relative and includes the package name; it's only when you mix in project references that you get the relative inferred specifiers.

[sheetalkamat]

This has nothing to do with project references but path mapping provided. If you remove reference to project, you are going to get same specifier. #26341 made sure that declaration emit uses relative paths all the time. If the paths are in node_modules, then and then only they use package name. Path mapping is not good enough indicator to use package name.

[ecraig12345]

Then what is the suggested way to handle this case, where we have project references as well as deep imports into other projects?

[robpalme]

@sheetalkamat 's fix in #42224 looks very good. It helps make declaration emit more intuitive and more controllable by the user, because it steers towards reflecting what the user wrote. It permits adhoc fix-ups by users because, worst case, if a synthetic (inferred) import is in an undesirable form, the user can now add their own preferred alias to the source code to tweak the declaration emit.

So it's a big step forward (and I think worth landing), but it would be awesome if we could later come up with a comprehensive solution.

---

If the paths are in node_modules, then and then only they use package name.

I understand that historically this has been a simple heuristic to determine if a file belongs to an external dependency or not. In future it would be good to avoid relying on this assumption because whilst it's a very popular pattern, node_modules is not a fundamental property of the language. There are other JS systems out there that unpack dependencies into other locations. And, even more common, there are monorepos where packages live side-by-side inside a directory that is not called node_modules.

In the system I work on, we solve this problem automatically for our users by post-processing the DTS files emitted by TypeScript to mop up any unwanted synthetic relative paths by converting them into a canonical form, i.e. package-prefixed bare specifiers. It works, but it sounds like @ecraig12345 has a similar desire, so maybe it's something that could be solved in TypeScript for everyone.

Path mapping is not good enough indicator to use package name.

For use-cases where dependencies do not live inside node_modules, import maps (as a concept) are one way to express the names and locations of dependencies. Which superficially looks similar to "paths" in tsconfig. This is probably why confusion arises - because when you survey tsconfig to answer the question of "how can I inform TypeScript about where to find my dependencies", "paths" seems to be the most obvious solution.

I think the simplest solution would be for declaration emit to read "paths" and use it to drive the preferred specifier selection. That's effectively what our post-processing transform achieves. It would be fine for this preference to have a lower precedence "reuse already imported specifiers" that you have implemented so far. Please say if you know of problems with this approach.

[sheetalkamat]

#42232 might help with project boundary issues but will have to see how it performs given we have so many different ways the file can be referenced. Will see how tests are running to get more idea. It doesnt break test case from #26341 for sure so thats a good start.

[Philipp91]

Thank you! I would like to test this. Normally the nightly releases appear by 8am UTC, but 4.2.0-dev.20210109 hasn't appeared yet. Anyway I'll just wait until there's a nightly release and then test it for our use case.

[ecraig12345]

I don't think this issue should be closed--while #42224 fixed the issue in some cases, it won't fix it in cases where the file isn't referenced in another import.

[Philipp91]

+1 to @ecraig12345's point. E.g. mui/material-ui#24112 is one of the cases that isn't solved because the symbol is only being used implicitly in the tsx file, so there's no explicit import that the heuristic can piggy-back on.

@robpalme

In the system I work on, we solve this problem automatically for our users by post-processing the DTS files emitted by TypeScript to mop up any unwanted synthetic relative paths by converting them into a canonical form, i.e. package-prefixed bare specifiers.

That sounds like a decent solution, as I imagine it breaks relatively rarely. Could you elaborate on how that post-processing works? Just some clever sed commands? Maybe you can share relevant snippets of your implementation, even if it's very specific to that project, so that others can adapt it?

[eps1lon]

@sheetalkamat Could you re-open this issue since #42232 did not fix all issues?

I can confirm that some problematic relative import paths are removed but some are still inserted.

Full diff of generated declaration files between 4.1.3 and 4.2.0-dev.20210112: https://app.circleci.com/pipelines/github/mui-org/material-ui/34050/workflows/861b9ce6-4157-442d-925c-e09f67a21790/jobs/213656/parallel-runs/0/steps/0-110

Example problematic import that is still generated: import("../../../material-ui-styles/src").ClassNameMap<PickersYearClassKey>;

[sheetalkamat]

Can you please open a new issue with the small repro that doesnt get fixed so we can reason about it as part of #42232

[eps1lon]

Can you please open a new issue with the small repro that doesnt get fixed so we can reason about it as part of #42232

@sheetalkamat I've seen that you can build an installable version of TypeScript from a PR. If you could do that I can quickly check it on our repo. A small repro might take longer.

[eps1lon]

@sheetalkamat Repro was quicker than expected: #42349