Skip to content

Inspect all possible module paths when looking for the best one to create a specifier with #25850

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,12 @@ namespace ts.moduleSpecifiers {
const symlinks = mapDefined(files, sf =>
sf.resolvedModules && firstDefinedIterator(sf.resolvedModules.values(), res =>
res && res.resolvedFileName === importedFileName ? res.originalPath : undefined));
return symlinks.length === 0 ? getAllModulePathsUsingIndirectSymlinks(files, getNormalizedAbsolutePath(importedFileName, host.getCurrentDirectory ? host.getCurrentDirectory() : ""), getCanonicalFileName, host) : symlinks;
const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : "";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow all the logic here, but there may be some duplicated effort. A few lines above this we get symlinks, then in discoverProbableSymlinks there is some nearly identical (copy-pasted?) code to do the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, it's not duplicated at all. Here we find files whose realpath differed from their actual path, provided those files refer to the imported file. In discoverProbableSymlinks, we crawl all files looking for common parts among all files with symlinks to find probable directory junction links. If I remember correctly, anyway.

const baseOptions = getAllModulePathsUsingIndirectSymlinks(files, getNormalizedAbsolutePath(importedFileName, cwd), getCanonicalFileName, host);
if (symlinks.length === 0) {
return baseOptions;
}
return deduplicate(concatenate(baseOptions, flatMap(symlinks, importedFileName => getAllModulePathsUsingIndirectSymlinks(files, getNormalizedAbsolutePath(importedFileName, cwd), getCanonicalFileName, host))));
}

function getRelativePathNParents(relativePath: string): number {
Expand Down
1 change: 1 addition & 0 deletions src/harness/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ namespace compiler {
public readonly js: ReadonlyMap<string, documents.TextDocument>;
public readonly dts: ReadonlyMap<string, documents.TextDocument>;
public readonly maps: ReadonlyMap<string, documents.TextDocument>;
public symlinks?: vfs.FileSet; // Location to store original symlinks so they may be used in both original and declaration file compilations

private _inputs: documents.TextDocument[] = [];
private _inputsAndOutputs: collections.SortedMap<string, CompilationOutput>;
Expand Down
13 changes: 8 additions & 5 deletions src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,9 @@ namespace Harness {
fs.apply(symlinks);
}
const host = new fakes.CompilerHost(fs, options);
return compiler.compileFiles(host, programFileNames, options);
const result = compiler.compileFiles(host, programFileNames, options);
result.symlinks = symlinks;
return result;
}

export interface DeclarationCompilationContext {
Expand Down Expand Up @@ -1261,7 +1263,7 @@ namespace Harness {
}

function addDtsFile(file: TestFile, dtsFiles: TestFile[]) {
if (vpath.isDeclaration(file.unitName)) {
if (vpath.isDeclaration(file.unitName) || vpath.isJson(file.unitName)) {
dtsFiles.push(file);
}
else if (vpath.isTypeScript(file.unitName)) {
Expand Down Expand Up @@ -1302,12 +1304,12 @@ namespace Harness {
}
}

export function compileDeclarationFiles(context: DeclarationCompilationContext | undefined) {
export function compileDeclarationFiles(context: DeclarationCompilationContext | undefined, symlinks: vfs.FileSet | undefined) {
if (!context) {
return;
}
const { declInputFiles, declOtherFiles, harnessSettings, options, currentDirectory } = context;
const output = compileFiles(declInputFiles, declOtherFiles, harnessSettings, options, currentDirectory);
const output = compileFiles(declInputFiles, declOtherFiles, harnessSettings, options, currentDirectory, symlinks);
return { declInputFiles, declOtherFiles, declResult: output };
}

Expand Down Expand Up @@ -1686,7 +1688,7 @@ namespace Harness {
const declFileContext = prepareDeclarationCompilationContext(
toBeCompiled, otherFiles, result, harnessSettings, options, /*currentDirectory*/ undefined
);
const declFileCompilationResult = compileDeclarationFiles(declFileContext);
const declFileCompilationResult = compileDeclarationFiles(declFileContext, result.symlinks);

if (declFileCompilationResult && declFileCompilationResult.declResult.diagnostics.length) {
jsCode += "\r\n\r\n//// [DtsFileErrors]\r\n";
Expand Down Expand Up @@ -1890,6 +1892,7 @@ namespace Harness {
for (const line of lines) {
let testMetaData: RegExpExecArray | null;
const linkMetaData = linkRegex.exec(line);
linkRegex.lastIndex = 0;
if (linkMetaData) {
if (!symlinks) symlinks = {};
symlinks[linkMetaData[2].trim()] = new vfs.Symlink(linkMetaData[1].trim());
Expand Down
3 changes: 2 additions & 1 deletion src/testRunner/rwcRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,9 @@ namespace RWC {
inputFiles, otherFiles, compilerResult, /*harnessSettings*/ undefined!, compilerOptions, currentDirectory // TODO: GH#18217
);
// Reset compilerResult before calling into `compileDeclarationFiles` so the memory from the original compilation can be freed
const links = compilerResult.symlinks;
compilerResult = undefined!;
const declFileCompilationResult = Harness.Compiler.compileDeclarationFiles(declContext)!;
const declFileCompilationResult = Harness.Compiler.compileDeclarationFiles(declContext, links)!;

return Harness.Compiler.iterateErrorBaseline(tsconfigFiles.concat(declFileCompilationResult.declInputFiles, declFileCompilationResult.declOtherFiles), declFileCompilationResult.declResult.diagnostics, { caseSensitive, currentDirectory });
}, baselineOpts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,3 @@ var Src = /** @class */ (function () {
/// <reference types="dep" />
declare class Src implements NS.Dep {
}


//// [DtsFileErrors]


error TS2688: Cannot find type definition file for 'dep'.
/src/index.d.ts(1,23): error TS2688: Cannot find type definition file for 'dep'.
/src/index.d.ts(2,30): error TS2503: Cannot find namespace 'NS'.


!!! error TS2688: Cannot find type definition file for 'dep'.
==== /src/index.d.ts (2 errors) ====
/// <reference types="dep" />
~~~
!!! error TS2688: Cannot find type definition file for 'dep'.
declare class Src implements NS.Dep {
~~
!!! error TS2503: Cannot find namespace 'NS'.
}

==== /deps/dep/dep.d.ts (0 errors) ====
declare namespace NS {
interface Dep {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,30 +44,3 @@ exports.default = parseArgs;
//// [index.d.ts]
import minimist = require('minimist');
export default function parseArgs(): minimist.ParsedArgs;


//// [DtsFileErrors]


error TS2688: Cannot find type definition file for 'minimist'.
error TS2688: Cannot find type definition file for 'process'.
/index.d.ts(1,27): error TS2307: Cannot find module 'minimist'.


!!! error TS2688: Cannot find type definition file for 'minimist'.
!!! error TS2688: Cannot find type definition file for 'process'.
==== /index.d.ts (1 errors) ====
import minimist = require('minimist');
~~~~~~~~~~
!!! error TS2307: Cannot find module 'minimist'.
export default function parseArgs(): minimist.ParsedArgs;

==== /node_modules/@types/minimist/minimist.d.ts (0 errors) ====
declare namespace thing {
interface ParsedArgs {}
}
declare function thing(x: any): thing.ParsedArgs;
export = thing;
==== /node_modules/@types/process/process.d.ts (0 errors) ====
declare const thing: any;
export = thing;
41 changes: 0 additions & 41 deletions tests/baselines/reference/symbolLinkDeclarationEmitModuleNames.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,44 +73,3 @@ export declare type ControllerClass = Constructor<any>;
//// [usage.d.ts]
import { BindingKey } from '@loopback/context';
export declare const CONTROLLER_CLASS: BindingKey<import("@loopback/context/src/value-promise").Constructor<any>>;


//// [DtsFileErrors]


tests/cases/compiler/monorepo/context/src/bindingkey.d.ts(1,29): error TS2307: Cannot find module '@loopback/context'.
tests/cases/compiler/monorepo/core/src/application.d.ts(1,29): error TS2307: Cannot find module '@loopback/context'.
tests/cases/compiler/monorepo/core/src/usage.d.ts(1,28): error TS2307: Cannot find module '@loopback/context'.
tests/cases/compiler/monorepo/core/src/usage.d.ts(2,58): error TS2307: Cannot find module '@loopback/context/src/value-promise'.


==== tests/cases/compiler/monorepo/core/src/application.d.ts (1 errors) ====
import { Constructor } from "@loopback/context";
~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module '@loopback/context'.
export declare type ControllerClass = Constructor<any>;

==== tests/cases/compiler/monorepo/core/src/usage.d.ts (2 errors) ====
import { BindingKey } from '@loopback/context';
~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module '@loopback/context'.
export declare const CONTROLLER_CLASS: BindingKey<import("@loopback/context/src/value-promise").Constructor<any>>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module '@loopback/context/src/value-promise'.

==== /.src/tests/cases/compiler/monorepo/context/src/value-promise.d.ts (0 errors) ====
export declare type Constructor<T> = (...args: any[]) => T;

==== /.src/tests/cases/compiler/monorepo/context/src/bindingkey.d.ts (1 errors) ====
import { Constructor } from "@loopback/context";
~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module '@loopback/context'.
export declare class BindingKey<T> {
readonly __type: T;
static create<T extends Constructor<any>>(ctor: T): BindingKey<T>;
}

==== /.src/tests/cases/compiler/monorepo/context/index.d.ts (0 errors) ====
export * from "./src/value-promise";
export * from "./src/bindingkey";

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//// [tests/cases/compiler/symbolLinkDeclarationEmitModuleNamesImportRef.ts] ////

//// [index.d.ts]
export declare const styles: import("styled-components").InterpolationValue[];

//// [package.json]
{
"name": "styled-components",
"version": "3.3.3",
"typings": "typings/styled-components.d.ts"
}

//// [styled-components.d.ts]
export interface InterpolationValue {}
//// [index.ts]
import { styles } from "package-a";

export function getStyles() {
return styles;
}


//// [index.js]
"use strict";
exports.__esModule = true;
var package_a_1 = require("package-a");
function getStyles() {
return package_a_1.styles;
}
exports.getStyles = getStyles;


//// [index.d.ts]
export declare function getStyles(): import("styled-components").InterpolationValue[];
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
=== tests/cases/compiler/Folder/monorepo/core/index.ts ===
import { styles } from "package-a";
>styles : Symbol(styles, Decl(index.ts, 0, 8))

export function getStyles() {
>getStyles : Symbol(getStyles, Decl(index.ts, 0, 35))

return styles;
>styles : Symbol(styles, Decl(index.ts, 0, 8))
}

=== tests/cases/compiler/Folder/monorepo/package-a/index.d.ts ===
export declare const styles: import("styled-components").InterpolationValue[];
>styles : Symbol(styles, Decl(index.d.ts, 0, 20))
>InterpolationValue : Symbol(InterpolationValue, Decl(styled-components.d.ts, 0, 0))

=== tests/cases/compiler/Folder/node_modules/styled-components/typings/styled-components.d.ts ===
export interface InterpolationValue {}
>InterpolationValue : Symbol(InterpolationValue, Decl(styled-components.d.ts, 0, 0))

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
=== tests/cases/compiler/Folder/monorepo/core/index.ts ===
import { styles } from "package-a";
>styles : import("tests/cases/compiler/Folder/node_modules/styled-components/typings/styled-components").InterpolationValue[]

export function getStyles() {
>getStyles : () => import("tests/cases/compiler/Folder/node_modules/styled-components/typings/styled-components").InterpolationValue[]

return styles;
>styles : import("tests/cases/compiler/Folder/node_modules/styled-components/typings/styled-components").InterpolationValue[]
}

=== tests/cases/compiler/Folder/monorepo/package-a/index.d.ts ===
export declare const styles: import("styled-components").InterpolationValue[];
>styles : import("tests/cases/compiler/Folder/node_modules/styled-components/typings/styled-components").InterpolationValue[]
>InterpolationValue : import("tests/cases/compiler/Folder/node_modules/styled-components/typings/styled-components").InterpolationValue

=== tests/cases/compiler/Folder/node_modules/styled-components/typings/styled-components.d.ts ===
export interface InterpolationValue {}
>InterpolationValue : InterpolationValue

Original file line number Diff line number Diff line change
Expand Up @@ -40,48 +40,3 @@ export declare type ControllerClass = Constructor<any>;
//// [usage.d.ts]
import { BindingKey } from '@loopback/context';
export declare const CONTROLLER_CLASS: BindingKey<import("@loopback/context/src/value-promise").Constructor<any>>;


//// [DtsFileErrors]


tests/cases/compiler/monorepo/core/dist/src/application.d.ts(1,29): error TS2307: Cannot find module '@loopback/context'.
tests/cases/compiler/monorepo/core/dist/src/usage.d.ts(1,28): error TS2307: Cannot find module '@loopback/context'.
tests/cases/compiler/monorepo/core/dist/src/usage.d.ts(2,58): error TS2307: Cannot find module '@loopback/context/src/value-promise'.


==== tests/cases/compiler/monorepo/core/tsconfig.json (0 errors) ====
{
"compilerOptions": {
"rootDir": ".",
"declaration": true,
"outDir": "./dist"
}
}
==== tests/cases/compiler/monorepo/context/src/value-promise.d.ts (0 errors) ====
export type Constructor<T> = (...args: any[]) => T;
==== tests/cases/compiler/monorepo/context/src/bindingkey.d.ts (0 errors) ====
import { Constructor } from "./value-promise"
export declare class BindingKey<T> {
readonly __type: T;
static create<T extends Constructor<any>>(ctor: T): BindingKey<T>;
}

==== tests/cases/compiler/monorepo/context/index.d.ts (0 errors) ====
export * from "./src/value-promise";
export * from "./src/bindingkey";

==== tests/cases/compiler/monorepo/core/dist/src/application.d.ts (1 errors) ====
import { Constructor } from "@loopback/context";
~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module '@loopback/context'.
export declare type ControllerClass = Constructor<any>;

==== tests/cases/compiler/monorepo/core/dist/src/usage.d.ts (2 errors) ====
import { BindingKey } from '@loopback/context';
~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module '@loopback/context'.
export declare const CONTROLLER_CLASS: BindingKey<import("@loopback/context/src/value-promise").Constructor<any>>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module '@loopback/context/src/value-promise'.

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// @declaration: true
// @useCaseSensitiveFileNames: false
// @noImplicitReferences: true
// @filename: Folder/monorepo/package-a/index.d.ts
export declare const styles: import("styled-components").InterpolationValue[];

// @filename: Folder/node_modules/styled-components/package.json
{
"name": "styled-components",
"version": "3.3.3",
"typings": "typings/styled-components.d.ts"
}

// @filename: Folder/node_modules/styled-components/typings/styled-components.d.ts
export interface InterpolationValue {}
// @filename: Folder/monorepo/core/index.ts
import { styles } from "package-a";

export function getStyles() {
return styles;
}

// @link: tests/cases/compiler/Folder/node_modules/styled-components -> tests/cases/compiler/Folder/monorepo/package-a/node_modules/styled-components
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like some tests use @link and some use @symlink, do you know why?

Copy link
Member Author

@weswigham weswigham Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@symlink links the below file to another filepath. @link links one place (like a folder) to another - like an ntfs junction.

@link is strictly more powerful - I added it because @symlink couldn't be used to repro the issues I was seeing.

// @link: tests/cases/compiler/Folder/monorepo/package-a -> tests/cases/compiler/Folder/monorepo/core/node_modules/package-a
3 changes: 3 additions & 0 deletions tests/cases/fourslash/importNameCodeFix_symlink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,8 @@ goTo.file("/b.ts");
verify.importFixAtPosition([
`import { foo } from "link";

foo;`,
`import { foo } from "real";

foo;`,
]);