Skip to content

Commit 4f50766

Browse files
weswighamweswigham
committed
Build better import paths from reexports if possible, issue error on node_modules import generation
1 parent e1c8dc2 commit 4f50766

29 files changed

+923
-30
lines changed

‎src/compiler/checker.ts

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2551,6 +2551,40 @@ namespace ts {
25512551
return getMergedSymbol(symbol.parent && getLateBoundSymbol(symbol.parent));
25522552
}
25532553

2554+
function getAlternativeContainingModules(symbol: Symbol, enclosingDeclaration: Node): Symbol[] | undefined {
2555+
const containingFile = getSourceFileOfNode(enclosingDeclaration);
2556+
if (containingFile && containingFile.imports) {
2557+
// Try to make an import using an import already in the enclosing file, if possible
2558+
let results: Symbol[] | undefined;
2559+
for (const importRef of containingFile.imports) {
2560+
if (nodeIsSynthesized(importRef)) continue; // Synthetic names can't be resolved by `resolveExternalModuleName` - they'll cause a debug assert if they error
2561+
const resolvedModule = resolveExternalModuleName(enclosingDeclaration, importRef);
2562+
if (!resolvedModule) continue;
2563+
const ref = getAliasForSymbolInContainer(resolvedModule, symbol);
2564+
if (!ref) continue;
2565+
results = append(results, resolvedModule);
2566+
}
2567+
if (results) {
2568+
return results;
2569+
}
2570+
}
2571+
const links = getSymbolLinks(symbol);
2572+
if (links.extendedContainers) {
2573+
return links.extendedContainers;
2574+
}
2575+
let results: Symbol[] = [];
2576+
// No results from files already being imported by this file - expand search (expensive, but not location-specific, so cached)
2577+
const otherFiles = host.getSourceFiles();
2578+
for (const file of otherFiles) {
2579+
if (!isExternalModule(file)) continue;
2580+
const sym = getSymbolOfNode(file);
2581+
const ref = getAliasForSymbolInContainer(sym, symbol);
2582+
if (!ref) continue;
2583+
results = append(results, sym);
2584+
}
2585+
return links.extendedContainers = results;
2586+
}
2587+
25542588
/**
25552589
* Attempts to find the symbol corresponding to the container a symbol is in - usually this
25562590
* is just its' `.parent`, but for locals, this value is `undefined`
@@ -2559,10 +2593,12 @@ namespace ts {
25592593
const container = getParentOfSymbol(symbol);
25602594
if (container) {
25612595
const additionalContainers = mapDefined(container.declarations, fileSymbolIfFileSymbolExportEqualsContainer);
2596+
const reexportContainers = enclosingDeclaration && getAlternativeContainingModules(symbol, enclosingDeclaration);
25622597
if (enclosingDeclaration && getAccessibleSymbolChain(container, enclosingDeclaration, SymbolFlags.Namespace, /*externalOnly*/ false)) {
2563-
return concatenate([container], additionalContainers); // This order expresses a preference for the real container if it is in scope
2598+
return concatenate(concatenate([container], additionalContainers), reexportContainers); // This order expresses a preference for the real container if it is in scope
25642599
}
2565-
return append(additionalContainers, container);
2600+
const res = append(additionalContainers, container);
2601+
return concatenate(res, reexportContainers);
25662602
}
25672603
const candidates = mapDefined(symbol.declarations, d => !isAmbientModule(d) && d.parent && hasNonGlobalAugmentationExternalModuleSymbol(d.parent) ? getSymbolOfNode(d.parent) : undefined);
25682604
if (!length(candidates)) {
@@ -3889,6 +3925,7 @@ namespace ts {
38893925
// Go up and add our parent.
38903926
const parents = getContainersOfSymbol(accessibleSymbolChain ? accessibleSymbolChain[0] : symbol, context.enclosingDeclaration);
38913927
if (length(parents)) {
3928+
parents!.sort(sortByBestName);
38923929
for (const parent of parents!) {
38933930
const parentChain = getSymbolChain(parent, getQualifiedLeftMeaning(meaning), /*endOfChain*/ false);
38943931
if (parentChain) {
@@ -3914,6 +3951,24 @@ namespace ts {
39143951
return [symbol];
39153952
}
39163953
}
3954+
3955+
function sortByBestName(a: Symbol, b: Symbol) {
3956+
if (some(a.declarations, hasNonGlobalAugmentationExternalModuleSymbol) && some(b.declarations, hasNonGlobalAugmentationExternalModuleSymbol)) {
3957+
const specifierA = getSpecifierForModuleSymbol(a, context);
3958+
const specifierB = getSpecifierForModuleSymbol(b, context);
3959+
if (pathIsRelative(specifierA) === pathIsRelative(specifierB)) {
3960+
// Both relative or both non-relative, sort by number of parts
3961+
return moduleSpecifiers.countPathComponents(specifierA) - moduleSpecifiers.countPathComponents(specifierB);
3962+
}
3963+
if (pathIsRelative(specifierB)) {
3964+
// A is non-relative, B is relative: prefer A
3965+
return -1;
3966+
}
3967+
// A is relative, B is non-relative: prefer B
3968+
return 1;
3969+
}
3970+
return 0;
3971+
}
39173972
}
39183973

39193974
function typeParametersToTypeParameterDeclarations(symbol: Symbol, context: NodeBuilderContext) {
@@ -4016,6 +4071,14 @@ namespace ts {
40164071
const nonRootParts = chain.length > 1 ? createAccessFromSymbolChain(chain, chain.length - 1, 1) : undefined;
40174072
const typeParameterNodes = overrideTypeArguments || lookupTypeParameterNodes(chain, 0, context);
40184073
const specifier = getSpecifierForModuleSymbol(chain[0], context);
4074+
if (!(context.flags & NodeBuilderFlags.AllowNodeModulesRelativePaths) && getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeJs && specifier.indexOf("/node_modules/") >= 0) {
4075+
// If ultimately we can only name the symbol with a reference that dives into a `node_modules` folder, we should error
4076+
// since declaration files with these kinds of references are liable to fail when published :(
4077+
context.encounteredError = true;
4078+
if (context.tracker.reportLikelyUnsafeImportRequiredError) {
4079+
context.tracker.reportLikelyUnsafeImportRequiredError(specifier);
4080+
}
4081+
}
40194082
const lit = createLiteralTypeNode(createLiteral(specifier));
40204083
if (context.tracker.trackExternalModuleSymbolOfImportTypeNode) context.tracker.trackExternalModuleSymbolOfImportTypeNode(chain[0]);
40214084
context.approximateLength += specifier.length + 10; // specifier + import("")

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2489,6 +2489,10 @@
24892489
"category": "Error",
24902490
"code": 2734
24912491
},
2492+
"The inferred type of '{0}' requires a reference to a module at '{1}' to name, which would likely break on type redistribution. A type annotation is necessary.": {
2493+
"category": "Error",
2494+
"code": 2735
2495+
},
24922496

24932497
"Import declaration '{0}' is using private name '{1}'.": {
24942498
"category": "Error",

‎src/compiler/moduleSpecifiers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ namespace ts.moduleSpecifiers {
139139
return isPathRelativeToParent(nonRelative) || countPathComponents(relativePath) < countPathComponents(nonRelative) ? relativePath : nonRelative;
140140
}
141141

142-
function countPathComponents(path: string): number {
142+
export function countPathComponents(path: string): number {
143143
let count = 0;
144144
for (let i = startsWith(path, "./") ? 2 : 0; i < path.length; i++) {
145145
if (path.charCodeAt(i) === CharacterCodes.slash) count++;

‎src/compiler/transformers/declarations.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ namespace ts {
4545
reportInaccessibleThisError,
4646
reportInaccessibleUniqueSymbolError,
4747
reportPrivateInBaseOfClassExpression,
48+
reportLikelyUnsafeImportRequiredError,
4849
moduleResolverHost: host,
4950
trackReferencedAmbientModule,
5051
trackExternalModuleSymbolOfImportTypeNode
@@ -153,6 +154,14 @@ namespace ts {
153154
}
154155
}
155156

157+
function reportLikelyUnsafeImportRequiredError(specifier: string) {
158+
if (errorNameNode) {
159+
context.addDiagnostic(createDiagnosticForNode(errorNameNode, Diagnostics.The_inferred_type_of_0_requires_a_reference_to_a_module_at_1_to_name_which_would_likely_break_on_type_redistribution_A_type_annotation_is_necessary,
160+
declarationNameToString(errorNameNode),
161+
specifier));
162+
}
163+
}
164+
156165
function transformRoot(node: Bundle): Bundle;
157166
function transformRoot(node: SourceFile): SourceFile;
158167
function transformRoot(node: SourceFile | Bundle): SourceFile | Bundle;

‎src/compiler/types.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3176,13 +3176,17 @@ namespace ts {
31763176
AllowUniqueESSymbolType = 1 << 20,
31773177
AllowEmptyIndexInfoType = 1 << 21,
31783178

3179-
IgnoreErrors = AllowThisInObjectLiteral | AllowQualifedNameInPlaceOfIdentifier | AllowAnonymousIdentifier | AllowEmptyUnionOrIntersection | AllowEmptyTuple | AllowEmptyIndexInfoType,
3179+
// Errors (cont.)
3180+
AllowNodeModulesRelativePaths = 1 << 26,
3181+
3182+
IgnoreErrors = AllowThisInObjectLiteral | AllowQualifedNameInPlaceOfIdentifier | AllowAnonymousIdentifier | AllowEmptyUnionOrIntersection | AllowEmptyTuple | AllowEmptyIndexInfoType | AllowNodeModulesRelativePaths,
31803183

31813184
// State
31823185
InObjectTypeLiteral = 1 << 22,
31833186
InTypeAlias = 1 << 23, // Writing type in type alias declaration
31843187
InInitialEntityName = 1 << 24, // Set when writing the LHS of an entity name or entity name expression
31853188
InReverseMappedType = 1 << 25,
3189+
31863190
}
31873191

31883192
// Ensure the shared flags between this and `NodeBuilderFlags` stay in alignment
@@ -3557,6 +3561,7 @@ namespace ts {
35573561
originatingImport?: ImportDeclaration | ImportCall; // Import declaration which produced the symbol, present if the symbol is marked as uncallable but had call signatures in `resolveESModuleSymbol`
35583562
lateSymbol?: Symbol; // Late-bound symbol for a computed property
35593563
specifierCache?: Map<string>; // For symbols corresponding to external modules, a cache of incoming path -> module specifier name mappings
3564+
extendedContainers?: Symbol[]; // Containers (other than the parent) which this symbol is aliased in
35603565
}
35613566

35623567
/* @internal */
@@ -5378,6 +5383,7 @@ namespace ts {
53785383
reportInaccessibleThisError?(): void;
53795384
reportPrivateInBaseOfClassExpression?(propertyName: string): void;
53805385
reportInaccessibleUniqueSymbolError?(): void;
5386+
reportLikelyUnsafeImportRequiredError?(specifier: string): void;
53815387
moduleResolverHost?: EmitHost;
53825388
trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void;
53835389
trackExternalModuleSymbolOfImportTypeNode?(symbol: Symbol): void;

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1979,7 +1979,8 @@ declare namespace ts {
19791979
AllowEmptyTuple = 524288,
19801980
AllowUniqueESSymbolType = 1048576,
19811981
AllowEmptyIndexInfoType = 2097152,
1982-
IgnoreErrors = 3112960,
1982+
AllowNodeModulesRelativePaths = 67108864,
1983+
IgnoreErrors = 70221824,
19831984
InObjectTypeLiteral = 4194304,
19841985
InTypeAlias = 8388608,
19851986
InInitialEntityName = 16777216,

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1979,7 +1979,8 @@ declare namespace ts {
19791979
AllowEmptyTuple = 524288,
19801980
AllowUniqueESSymbolType = 1048576,
19811981
AllowEmptyIndexInfoType = 2097152,
1982-
IgnoreErrors = 3112960,
1982+
AllowNodeModulesRelativePaths = 67108864,
1983+
IgnoreErrors = 70221824,
19831984
InObjectTypeLiteral = 4194304,
19841985
InTypeAlias = 8388608,
19851986
InInitialEntityName = 16777216,

‎tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.errors.txt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
tests/cases/compiler/r/entry.ts(3,14): error TS2735: The inferred type of 'x' requires a reference to a module at 'foo/node_modules/nested' to name, which would likely break on type redistribution. A type annotation is necessary.
2+
3+
4+
==== tests/cases/compiler/r/node_modules/foo/node_modules/nested/index.d.ts (0 errors) ====
5+
export interface NestedProps {}
6+
==== tests/cases/compiler/r/node_modules/foo/other/index.d.ts (0 errors) ====
7+
export interface OtherIndexProps {}
8+
==== tests/cases/compiler/r/node_modules/foo/other.d.ts (0 errors) ====
9+
export interface OtherProps {}
10+
==== tests/cases/compiler/r/node_modules/foo/index.d.ts (0 errors) ====
11+
import { OtherProps } from "./other";
12+
import { OtherIndexProps } from "./other/index";
13+
import { NestedProps } from "nested";
14+
export interface SomeProps {}
15+
16+
export function foo(): [SomeProps, OtherProps, OtherIndexProps, NestedProps];
17+
==== tests/cases/compiler/node_modules/root/index.d.ts (0 errors) ====
18+
export interface RootProps {}
19+
20+
export function bar(): RootProps;
21+
==== tests/cases/compiler/r/entry.ts (1 errors) ====
22+
import { foo } from "foo";
23+
import { bar } from "root";
24+
export const x = foo();
25+
~
26+
!!! error TS2735: The inferred type of 'x' requires a reference to a module at 'foo/node_modules/nested' to name, which would likely break on type redistribution. A type annotation is necessary.
27+
export const y = bar();
28+

‎tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.js

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,3 @@ var foo_1 = require("foo");
3131
var root_1 = require("root");
3232
exports.x = foo_1.foo();
3333
exports.y = root_1.bar();
34-
35-
36-
//// [entry.d.ts]
37-
export declare const x: [import("foo").SomeProps, import("foo/other").OtherProps, import("foo/other/index").OtherIndexProps, import("foo/node_modules/nested").NestedProps];
38-
export declare const y: import("root").RootProps;

‎tests/baselines/reference/declarationEmitReexportedSymlinkReference.js

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//// [tests/cases/compiler/declarationEmitReexportedSymlinkReference.ts] ////
2+
3+
//// [index.d.ts]
4+
export * from './types';
5+
//// [types.d.ts]
6+
export declare type A = {
7+
id: string;
8+
};
9+
export declare type B = {
10+
id: number;
11+
};
12+
export declare type IdType = A | B;
13+
export declare class MetadataAccessor<T, D extends IdType = IdType> {
14+
readonly key: string;
15+
private constructor();
16+
toString(): string;
17+
static create<T, D extends IdType = IdType>(key: string): MetadataAccessor<T, D>;
18+
}
19+
//// [package.json]
20+
{
21+
"name": "@raymondfeng/pkg1",
22+
"version": "1.0.0",
23+
"description": "",
24+
"main": "dist/index.js",
25+
"typings": "dist/index.d.ts"
26+
}
27+
//// [index.d.ts]
28+
export * from './types';
29+
//// [types.d.ts]
30+
export * from '@raymondfeng/pkg1';
31+
//// [package.json]
32+
{
33+
"name": "@raymondfeng/pkg2",
34+
"version": "1.0.0",
35+
"description": "",
36+
"main": "dist/index.js",
37+
"typings": "dist/index.d.ts"
38+
}
39+
//// [index.ts]
40+
export * from './keys';
41+
//// [keys.ts]
42+
import {MetadataAccessor} from "@raymondfeng/pkg2";
43+
44+
export const ADMIN = MetadataAccessor.create<boolean>('1');
45+
46+
//// [keys.js]
47+
"use strict";
48+
Object.defineProperty(exports, "__esModule", { value: true });
49+
var pkg2_1 = require("@raymondfeng/pkg2");
50+
exports.ADMIN = pkg2_1.MetadataAccessor.create('1');
51+
//// [index.js]
52+
"use strict";
53+
function __export(m) {
54+
for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p];
55+
}
56+
Object.defineProperty(exports, "__esModule", { value: true });
57+
__export(require("./keys"));
58+
59+
60+
//// [keys.d.ts]
61+
import { MetadataAccessor } from "@raymondfeng/pkg2";
62+
export declare const ADMIN: MetadataAccessor<boolean, import("@raymondfeng/pkg2").IdType>;
63+
//// [index.d.ts]
64+
export * from './keys';

‎tests/baselines/reference/declarationEmitReexportedSymlinkReference.symbols

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
=== tests/cases/compiler/monorepo/pkg1/dist/index.d.ts ===
2+
export * from './types';
3+
No type information for this code.=== tests/cases/compiler/monorepo/pkg1/dist/types.d.ts ===
4+
export declare type A = {
5+
>A : Symbol(A, Decl(types.d.ts, 0, 0))
6+
7+
id: string;
8+
>id : Symbol(id, Decl(types.d.ts, 0, 25))
9+
10+
};
11+
export declare type B = {
12+
>B : Symbol(B, Decl(types.d.ts, 2, 2))
13+
14+
id: number;
15+
>id : Symbol(id, Decl(types.d.ts, 3, 25))
16+
17+
};
18+
export declare type IdType = A | B;
19+
>IdType : Symbol(IdType, Decl(types.d.ts, 5, 2))
20+
>A : Symbol(A, Decl(types.d.ts, 0, 0))
21+
>B : Symbol(B, Decl(types.d.ts, 2, 2))
22+
23+
export declare class MetadataAccessor<T, D extends IdType = IdType> {
24+
>MetadataAccessor : Symbol(MetadataAccessor, Decl(types.d.ts, 6, 35))
25+
>T : Symbol(T, Decl(types.d.ts, 7, 38))
26+
>D : Symbol(D, Decl(types.d.ts, 7, 40))
27+
>IdType : Symbol(IdType, Decl(types.d.ts, 5, 2))
28+
>IdType : Symbol(IdType, Decl(types.d.ts, 5, 2))
29+
30+
readonly key: string;
31+
>key : Symbol(MetadataAccessor.key, Decl(types.d.ts, 7, 69))
32+
33+
private constructor();
34+
toString(): string;
35+
>toString : Symbol(MetadataAccessor.toString, Decl(types.d.ts, 9, 26))
36+
37+
static create<T, D extends IdType = IdType>(key: string): MetadataAccessor<T, D>;
38+
>create : Symbol(MetadataAccessor.create, Decl(types.d.ts, 10, 23))
39+
>T : Symbol(T, Decl(types.d.ts, 11, 18))
40+
>D : Symbol(D, Decl(types.d.ts, 11, 20))
41+
>IdType : Symbol(IdType, Decl(types.d.ts, 5, 2))
42+
>IdType : Symbol(IdType, Decl(types.d.ts, 5, 2))
43+
>key : Symbol(key, Decl(types.d.ts, 11, 48))
44+
>MetadataAccessor : Symbol(MetadataAccessor, Decl(types.d.ts, 6, 35))
45+
>T : Symbol(T, Decl(types.d.ts, 11, 18))
46+
>D : Symbol(D, Decl(types.d.ts, 11, 20))
47+
}
48+
=== tests/cases/compiler/monorepo/pkg2/dist/index.d.ts ===
49+
export * from './types';
50+
No type information for this code.=== tests/cases/compiler/monorepo/pkg2/dist/types.d.ts ===
51+
export * from '@raymondfeng/pkg1';
52+
No type information for this code.=== tests/cases/compiler/monorepo/pkg3/src/index.ts ===
53+
export * from './keys';
54+
No type information for this code.=== tests/cases/compiler/monorepo/pkg3/src/keys.ts ===
55+
import {MetadataAccessor} from "@raymondfeng/pkg2";
56+
>MetadataAccessor : Symbol(MetadataAccessor, Decl(keys.ts, 0, 8))
57+
58+
export const ADMIN = MetadataAccessor.create<boolean>('1');
59+
>ADMIN : Symbol(ADMIN, Decl(keys.ts, 2, 12))
60+
>MetadataAccessor.create : Symbol(MetadataAccessor.create, Decl(types.d.ts, 10, 23))
61+
>MetadataAccessor : Symbol(MetadataAccessor, Decl(keys.ts, 0, 8))
62+
>create : Symbol(MetadataAccessor.create, Decl(types.d.ts, 10, 23))
63+

0 commit comments

Comments
 (0)