Skip to content

Commit

Permalink
Merge pull request #33567 from microsoft/resolution
Browse files Browse the repository at this point in the history
  Sort the paths for module specifier by closeness to importing file path
  • Loading branch information
sheetalkamat authored Sep 25, 2019
2 parents 250d5a8 + 91c66a0 commit 3dd7b84
Show file tree
Hide file tree
Showing 8 changed files with 253 additions and 62 deletions.
39 changes: 38 additions & 1 deletion src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,18 @@ namespace ts.moduleSpecifiers {
return result;
}

function numberOfDirectorySeparators(str: string) {
const match = str.match(/\//g);
return match ? match.length : 0;
}

function comparePathsByNumberOfDirectrorySeparators(a: string, b: string) {
return compareValues(
numberOfDirectorySeparators(a),
numberOfDirectorySeparators(b)
);
}

/**
* Looks for existing imports that use symlinks to this module.
* Symlinks will be returned first so they are preferred over the real path.
Expand Down Expand Up @@ -214,7 +226,32 @@ namespace ts.moduleSpecifiers {
}
});
result.push(...targets);
return result;
if (result.length < 2) return result;

// Sort by paths closest to importing file Name directory
const allFileNames = arrayToMap(result, identity, getCanonicalFileName);
const sortedPaths: string[] = [];
for (
let directory = getDirectoryPath(toPath(importingFileName, cwd, getCanonicalFileName));
allFileNames.size !== 0;
directory = getDirectoryPath(directory)
) {
const directoryStart = ensureTrailingDirectorySeparator(directory);
let pathsInDirectory: string[] | undefined;
allFileNames.forEach((canonicalFileName, fileName) => {
if (startsWith(canonicalFileName, directoryStart)) {
(pathsInDirectory || (pathsInDirectory = [])).push(fileName);
allFileNames.delete(fileName);
}
});
if (pathsInDirectory) {
if (pathsInDirectory.length > 1) {
pathsInDirectory.sort(comparePathsByNumberOfDirectrorySeparators);
}
sortedPaths.push(...pathsInDirectory);
}
}
return sortedPaths;
}

function tryGetModuleNameFromAmbientModule(moduleSymbol: Symbol): string | undefined {
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ namespace ts {
export function listFiles(program: ProgramToEmitFilesAndReportErrors, writeFileName: (s: string) => void) {
if (program.getCompilerOptions().listFiles) {
forEach(program.getSourceFiles(), file => {
writeFileName(file.fileName);
writeFileName(
!file.redirectInfo ?
file.fileName :
`${file.fileName} -> ${file.redirectInfo.redirectTarget.fileName}`
);
});
}
}
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
"unittests/tsbuild/transitiveReferences.ts",
"unittests/tsbuild/watchEnvironment.ts",
"unittests/tsbuild/watchMode.ts",
"unittests/tsc/declarationEmit.ts",
"unittests/tscWatch/consoleClearing.ts",
"unittests/tscWatch/emit.ts",
"unittests/tscWatch/emitAndErrorUpdates.ts",
Expand Down
136 changes: 136 additions & 0 deletions src/testRunner/unittests/tsc/declarationEmit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
namespace ts {
describe("unittests:: tsc:: declarationEmit::", () => {
verifyTsc({
scenario: "declarationEmit",
subScenario: "when same version is referenced through source and another symlinked package",
fs: () => {
const fsaPackageJson = utils.dedent`
{
"name": "typescript-fsa",
"version": "3.0.0-beta-2"
}`;
const fsaIndex = utils.dedent`
export interface Action<Payload> {
type: string;
payload: Payload;
}
export declare type ActionCreator<Payload> = {
type: string;
(payload: Payload): Action<Payload>;
}
export interface ActionCreatorFactory {
<Payload = void>(type: string): ActionCreator<Payload>;
}
export declare function actionCreatorFactory(prefix?: string | null): ActionCreatorFactory;
export default actionCreatorFactory;`;
return loadProjectFromFiles({
"/src/plugin-two/index.d.ts": utils.dedent`
declare const _default: {
features: {
featureOne: {
actions: {
featureOne: {
(payload: {
name: string;
order: number;
}, meta?: {
[key: string]: any;
}): import("typescript-fsa").Action<{
name: string;
order: number;
}>;
};
};
path: string;
};
};
};
export default _default;`,
"/src/plugin-two/node_modules/typescript-fsa/package.json": fsaPackageJson,
"/src/plugin-two/node_modules/typescript-fsa/index.d.ts": fsaIndex,
"/src/plugin-one/tsconfig.json": utils.dedent`
{
"compilerOptions": {
"target": "es5",
"declaration": true,
},
}`,
"/src/plugin-one/index.ts": utils.dedent`
import pluginTwo from "plugin-two"; // include this to add reference to symlink`,
"/src/plugin-one/action.ts": utils.dedent`
import { actionCreatorFactory } from "typescript-fsa"; // Include version of shared lib
const action = actionCreatorFactory("somekey");
const featureOne = action<{ route: string }>("feature-one");
export const actions = { featureOne };`,
"/src/plugin-one/node_modules/typescript-fsa/package.json": fsaPackageJson,
"/src/plugin-one/node_modules/typescript-fsa/index.d.ts": fsaIndex,
"/src/plugin-one/node_modules/plugin-two": new vfs.Symlink("/src/plugin-two"),
});
},
commandLineArgs: ["-p", "src/plugin-one", "--listFiles"]
});

verifyTsc({
scenario: "declarationEmit",
subScenario: "when pkg references sibling package through indirect symlink",
fs: () => loadProjectFromFiles({
"/src/pkg1/dist/index.d.ts": utils.dedent`
export * from './types';`,
"/src/pkg1/dist/types.d.ts": utils.dedent`
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>;
}`,
"/src/pkg1/package.json": utils.dedent`
{
"name": "@raymondfeng/pkg1",
"version": "1.0.0",
"description": "",
"main": "dist/index.js",
"typings": "dist/index.d.ts"
}`,
"/src/pkg2/dist/index.d.ts": utils.dedent`
export * from './types';`,
"/src/pkg2/dist/types.d.ts": utils.dedent`
export {MetadataAccessor} from '@raymondfeng/pkg1';`,
"/src/pkg2/package.json": utils.dedent`
{
"name": "@raymondfeng/pkg2",
"version": "1.0.0",
"description": "",
"main": "dist/index.js",
"typings": "dist/index.d.ts"
}`,
"/src/pkg3/src/index.ts": utils.dedent`
export * from './keys';`,
"/src/pkg3/src/keys.ts": utils.dedent`
import {MetadataAccessor} from "@raymondfeng/pkg2";
export const ADMIN = MetadataAccessor.create<boolean>('1');`,
"/src/pkg3/tsconfig.json": utils.dedent`
{
"compilerOptions": {
"outDir": "dist",
"rootDir": "src",
"target": "es5",
"module": "commonjs",
"strict": true,
"esModuleInterop": true,
"declaration": true
}
}`,
"/src/pkg2/node_modules/@raymondfeng/pkg1": new vfs.Symlink("/src/pkg1"),
"/src/pkg3/node_modules/@raymondfeng/pkg2": new vfs.Symlink("/src/pkg2"),
}),
commandLineArgs: ["-p", "src/pkg3", "--listFiles"]
});
});
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,8 @@ Object.defineProperty(exports, "__esModule", { value: true });
__export(require("./keys"));


//// [keys.d.ts]
import { MetadataAccessor } from "@raymondfeng/pkg2";
export declare const ADMIN: MetadataAccessor<boolean, import("../../pkg1/dist").IdType>;
//// [index.d.ts]
export * from './keys';
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//// [/lib/initial-buildOutput.txt]
/lib/tsc -p src/pkg3 --listFiles
src/pkg3/src/keys.ts(2,14): error TS2742: The inferred type of 'ADMIN' cannot be named without a reference to '@raymondfeng/pkg2/node_modules/@raymondfeng/pkg1'. This is likely not portable. A type annotation is necessary.
/lib/lib.d.ts
/src/pkg3/node_modules/@raymondfeng/pkg2/node_modules/@raymondfeng/pkg1/dist/types.d.ts
/src/pkg3/node_modules/@raymondfeng/pkg2/node_modules/@raymondfeng/pkg1/dist/index.d.ts
/src/pkg3/node_modules/@raymondfeng/pkg2/dist/types.d.ts
/src/pkg3/node_modules/@raymondfeng/pkg2/dist/index.d.ts
/src/pkg3/src/keys.ts
/src/pkg3/src/index.ts
exitCode:: 1


//// [/src/pkg3/dist/index.d.ts]
export * from './keys';


//// [/src/pkg3/dist/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"));


//// [/src/pkg3/dist/keys.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var pkg2_1 = require("@raymondfeng/pkg2");
exports.ADMIN = pkg2_1.MetadataAccessor.create('1');


Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//// [/lib/initial-buildOutput.txt]
/lib/tsc -p src/plugin-one --listFiles
/lib/lib.d.ts
/src/plugin-one/node_modules/typescript-fsa/index.d.ts
/src/plugin-one/action.ts
/src/plugin-one/node_modules/plugin-two/node_modules/typescript-fsa/index.d.ts -> /src/plugin-one/node_modules/typescript-fsa/index.d.ts
/src/plugin-one/node_modules/plugin-two/index.d.ts
/src/plugin-one/index.ts
exitCode:: 0


//// [/src/plugin-one/action.d.ts]
export declare const actions: {
featureOne: import("typescript-fsa").ActionCreator<{
route: string;
}>;
};


//// [/src/plugin-one/action.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var typescript_fsa_1 = require("typescript-fsa"); // Include version of shared lib
var action = typescript_fsa_1.actionCreatorFactory("somekey");
var featureOne = action("feature-one");
exports.actions = { featureOne: featureOne };


//// [/src/plugin-one/index.d.ts]
export {};


//// [/src/plugin-one/index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });


0 comments on commit 3dd7b84

Please sign in to comment.