Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 13.7k
Build better import paths for declaration emit/typeToString from reexports if possible#27340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
4f50766a4a11f99845f8af18e5b2a04077cc053413278149ca8dd3a6File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2599,6 +2599,44 @@ namespace ts { | ||
| return getMergedSymbol(symbol.parent && getLateBoundSymbol(symbol.parent)); | ||
| } | ||
| function getAlternativeContainingModules(symbol: Symbol, enclosingDeclaration: Node): Symbol[] { | ||
| const containingFile = getSourceFileOfNode(enclosingDeclaration); | ||
| const id = "" + getNodeId(containingFile); | ||
| const links = getSymbolLinks(symbol); | ||
| let results: Symbol[] | undefined; | ||
weswigham marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (links.extendedContainersByFile && (results = links.extendedContainersByFile.get(id))) { | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This nesting is unnecessarily complicated. What about this: letresults=links.extendedContainersByFile&&links.extendedContainersByFile.get(id);if(results)returnresults; | ||
| return results; | ||
| } | ||
| if (containingFile && containingFile.imports) { | ||
| // Try to make an import using an import already in the enclosing file, if possible | ||
| for (const importRef of containingFile.imports) { | ||
| if (nodeIsSynthesized(importRef)) continue; // Synthetic names can't be resolved by `resolveExternalModuleName` - they'll cause a debug assert if they error | ||
| const resolvedModule = resolveExternalModuleName(enclosingDeclaration, importRef); | ||
| if (!resolvedModule) continue; | ||
| const ref = getAliasForSymbolInContainer(resolvedModule, symbol); | ||
| if (!ref) continue; | ||
| results = append(results, resolvedModule); | ||
| } | ||
| if (length(results)) { | ||
| (links.extendedContainersByFile || (links.extendedContainersByFile = createMap())).set(id, results!); | ||
| return results!; | ||
| } | ||
| } | ||
| if (links.extendedContainers) { | ||
| return links.extendedContainers; | ||
| } | ||
| // No results from files already being imported by this file - expand search (expensive, but not location-specific, so cached) | ||
| const otherFiles = host.getSourceFiles(); | ||
| for (const file of otherFiles) { | ||
| if (!isExternalModule(file)) continue; | ||
| const sym = getSymbolOfNode(file); | ||
| const ref = getAliasForSymbolInContainer(sym, symbol); | ||
| if (!ref) continue; | ||
| results = append(results, sym); | ||
| } | ||
| return links.extendedContainers = results || emptyArray; | ||
| } | ||
| /** | ||
| * Attempts to find the symbol corresponding to the container a symbol is in - usually this | ||
| * is just its' `.parent`, but for locals, this value is `undefined` | ||
| @@ -2607,10 +2645,12 @@ namespace ts { | ||
| const container = getParentOfSymbol(symbol); | ||
| if (container) { | ||
| const additionalContainers = mapDefined(container.declarations, fileSymbolIfFileSymbolExportEqualsContainer); | ||
| const reexportContainers = enclosingDeclaration && getAlternativeContainingModules(symbol, enclosingDeclaration); | ||
| if (enclosingDeclaration && getAccessibleSymbolChain(container, enclosingDeclaration, SymbolFlags.Namespace, /*externalOnly*/ false)) { | ||
| return concatenate([container], additionalContainers); // This order expresses a preference for the real container if it is in scope | ||
| return concatenate(concatenate([container], additionalContainers), reexportContainers); // This order expresses a preference for the real container if it is in scope | ||
| } | ||
| return append(additionalContainers, container); | ||
| const res = append(additionalContainers, container); | ||
| return concatenate(res, reexportContainers); | ||
| } | ||
| const candidates = mapDefined(symbol.declarations, d => !isAmbientModule(d) && d.parent && hasNonGlobalAugmentationExternalModuleSymbol(d.parent) ? getSymbolOfNode(d.parent) : undefined); | ||
| if (!length(candidates)) { | ||
| @@ -3951,14 +3991,21 @@ namespace ts { | ||
| /** @param endOfChain Set to false for recursive calls; non-recursive calls should always output something. */ | ||
| function getSymbolChain(symbol: Symbol, meaning: SymbolFlags, endOfChain: boolean): Symbol[] | undefined { | ||
weswigham marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| let accessibleSymbolChain = getAccessibleSymbolChain(symbol, context.enclosingDeclaration, meaning, !!(context.flags & NodeBuilderFlags.UseOnlyExternalAliasing)); | ||
| let parentSpecifiers: (string | undefined)[]; | ||
| if (!accessibleSymbolChain || | ||
| needsQualification(accessibleSymbolChain[0], context.enclosingDeclaration, accessibleSymbolChain.length === 1 ? meaning : getQualifiedLeftMeaning(meaning))) { | ||
| // Go up and add our parent. | ||
| const parents = getContainersOfSymbol(accessibleSymbolChain ? accessibleSymbolChain[0] : symbol, context.enclosingDeclaration); | ||
| if (length(parents)) { | ||
| for (const parent of parents!) { | ||
| parentSpecifiers = parents!.map(symbol => | ||
| some(symbol.declarations, hasNonGlobalAugmentationExternalModuleSymbol) | ||
| ? getSpecifierForModuleSymbol(symbol, context) | ||
| : undefined); | ||
| const indices = parents!.map((_, i) => i); | ||
| indices.sort(sortByBestName); | ||
| const sortedParents = indices.map(i => parents![i]); | ||
| for (const parent of sortedParents) { | ||
| const parentChain = getSymbolChain(parent, getQualifiedLeftMeaning(meaning), /*endOfChain*/ false); | ||
| if (parentChain) { | ||
| accessibleSymbolChain = parentChain.concat(accessibleSymbolChain || [getAliasForSymbolInContainer(parent, symbol) || symbol]); | ||
| @@ -3982,6 +4029,25 @@ namespace ts { | ||
| } | ||
| return [symbol]; | ||
| } | ||
| function sortByBestName(a: number, b: number) { | ||
| const specifierA = parentSpecifiers[a]; | ||
| const specifierB = parentSpecifiers[b]; | ||
weswigham marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| if (specifierA && specifierB) { | ||
| const isBRelative = pathIsRelative(specifierB); | ||
| if (pathIsRelative(specifierA) === isBRelative) { | ||
| // Both relative or both non-relative, sort by number of parts | ||
| return moduleSpecifiers.countPathComponents(specifierA) - moduleSpecifiers.countPathComponents(specifierB); | ||
| } | ||
| if (isBRelative) { | ||
| // A is non-relative, B is relative: prefer A | ||
| return -1; | ||
| } | ||
| // A is relative, B is non-relative: prefer B | ||
| return 1; | ||
| } | ||
| return 0; | ||
| } | ||
| } | ||
| } | ||
| @@ -4085,6 +4151,14 @@ namespace ts { | ||
| const nonRootParts = chain.length > 1 ? createAccessFromSymbolChain(chain, chain.length - 1, 1) : undefined; | ||
| const typeParameterNodes = overrideTypeArguments || lookupTypeParameterNodes(chain, 0, context); | ||
| const specifier = getSpecifierForModuleSymbol(chain[0], context); | ||
| if (!(context.flags & NodeBuilderFlags.AllowNodeModulesRelativePaths) && getEmitModuleResolutionKind(compilerOptions) === ModuleResolutionKind.NodeJs && specifier.indexOf("/node_modules/") >= 0) { | ||
| // If ultimately we can only name the symbol with a reference that dives into a `node_modules` folder, we should error | ||
| // since declaration files with these kinds of references are liable to fail when published :( | ||
| context.encounteredError = true; | ||
| if (context.tracker.reportLikelyUnsafeImportRequiredError) { | ||
| context.tracker.reportLikelyUnsafeImportRequiredError(specifier); | ||
| } | ||
| } | ||
| const lit = createLiteralTypeNode(createLiteral(specifier)); | ||
| if (context.tracker.trackExternalModuleSymbolOfImportTypeNode) context.tracker.trackExternalModuleSymbolOfImportTypeNode(chain[0]); | ||
| context.approximateLength += specifier.length + 10; // specifier + import("") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| tests/cases/compiler/r/entry.ts(3,14): error TS2742: The inferred type of 'x' cannot be named without a reference to 'foo/node_modules/nested'. This is likely not portable. A type annotation is necessary. | ||
| ==== tests/cases/compiler/r/node_modules/foo/node_modules/nested/index.d.ts (0 errors) ==== | ||
| export interface NestedProps {} | ||
| ==== tests/cases/compiler/r/node_modules/foo/other/index.d.ts (0 errors) ==== | ||
| export interface OtherIndexProps {} | ||
| ==== tests/cases/compiler/r/node_modules/foo/other.d.ts (0 errors) ==== | ||
| export interface OtherProps {} | ||
| ==== tests/cases/compiler/r/node_modules/foo/index.d.ts (0 errors) ==== | ||
| import { OtherProps } from "./other"; | ||
| import { OtherIndexProps } from "./other/index"; | ||
| import { NestedProps } from "nested"; | ||
| export interface SomeProps {} | ||
| export function foo(): [SomeProps, OtherProps, OtherIndexProps, NestedProps]; | ||
| ==== tests/cases/compiler/node_modules/root/index.d.ts (0 errors) ==== | ||
| export interface RootProps {} | ||
| export function bar(): RootProps; | ||
| ==== tests/cases/compiler/r/entry.ts (1 errors) ==== | ||
| import { foo } from "foo"; | ||
| import { bar } from "root"; | ||
| export const x = foo(); | ||
| ~ | ||
| !!! error TS2742: The inferred type of 'x' cannot be named without a reference to 'foo/node_modules/nested'. This is likely not portable. A type annotation is necessary. | ||
| export const y = bar(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| //// [tests/cases/compiler/declarationEmitReexportedSymlinkReference.ts] //// | ||
| //// [index.d.ts] | ||
| export * from './types'; | ||
| //// [types.d.ts] | ||
| export declare type A = { | ||
| id: string; | ||
| }; | ||
| export declare type B = { | ||
| id: number; | ||
| }; | ||
| export declare type IdType = A | B; | ||
| export declare class MetadataAccessor<T, D extends IdType = IdType> { | ||
| readonly key: string; | ||
| private constructor(); | ||
| toString(): string; | ||
| static create<T, D extends IdType = IdType>(key: string): MetadataAccessor<T, D>; | ||
| } | ||
| //// [package.json] | ||
| { | ||
| "name": "@raymondfeng/pkg1", | ||
| "version": "1.0.0", | ||
| "description": "", | ||
| "main": "dist/index.js", | ||
| "typings": "dist/index.d.ts" | ||
| } | ||
| //// [index.d.ts] | ||
| export * from './types'; | ||
| //// [types.d.ts] | ||
| export * from '@raymondfeng/pkg1'; | ||
| //// [package.json] | ||
| { | ||
| "name": "@raymondfeng/pkg2", | ||
| "version": "1.0.0", | ||
| "description": "", | ||
| "main": "dist/index.js", | ||
| "typings": "dist/index.d.ts" | ||
| } | ||
| //// [index.ts] | ||
| export * from './keys'; | ||
| //// [keys.ts] | ||
| import {MetadataAccessor} from "@raymondfeng/pkg2"; | ||
| export const ADMIN = MetadataAccessor.create<boolean>('1'); | ||
| //// [keys.js] | ||
| "use strict"; | ||
| Object.defineProperty(exports, "__esModule", { value: true }); | ||
| var pkg2_1 = require("@raymondfeng/pkg2"); | ||
| exports.ADMIN = pkg2_1.MetadataAccessor.create('1'); | ||
| //// [index.js] | ||
| "use strict"; | ||
| function __export(m) { | ||
| for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p]; | ||
| } | ||
| Object.defineProperty(exports, "__esModule", { value: true }); | ||
| __export(require("./keys")); | ||
| //// [keys.d.ts] | ||
| import { MetadataAccessor } from "@raymondfeng/pkg2"; | ||
| export declare const ADMIN: MetadataAccessor<boolean, import("@raymondfeng/pkg2").IdType>; | ||
| //// [index.d.ts] | ||
| export * from './keys'; |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.