Skip to content

Handle packages inside another node modules package when auto importing #37561

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.

Sign up for GitHub

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
merged 1 commit into from
Mar 25, 2020
Merged

Handle packages inside another node modules package when auto importing #37561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 46 additions & 27 deletions src/compiler/moduleSpecifiers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,33 +319,30 @@ namespace ts.moduleSpecifiers {
return undefined;
}

let packageJsonContent: any | undefined;
const packageRootPath = moduleFileName.substring(0, parts.packageRootIndex);
// Simplify the full file path to something that can be resolved by Node.

let moduleSpecifier = moduleFileName;
if (!packageNameOnly) {
const packageJsonPath = combinePaths(packageRootPath, "package.json");
packageJsonContent = host.fileExists(packageJsonPath)
? JSON.parse(host.readFile(packageJsonPath)!)
: undefined;
const versionPaths = packageJsonContent && packageJsonContent.typesVersions
? getPackageJsonTypesVersionsPaths(packageJsonContent.typesVersions)
: undefined;
if (versionPaths) {
const subModuleName = moduleFileName.slice(parts.packageRootIndex + 1);
const fromPaths = tryGetModuleNameFromPaths(
removeFileExtension(subModuleName),
removeExtensionAndIndexPostFix(subModuleName, Ending.Minimal, options),
versionPaths.paths
);
if (fromPaths !== undefined) {
moduleFileName = combinePaths(moduleFileName.slice(0, parts.packageRootIndex), fromPaths);
let packageRootIndex = parts.packageRootIndex;
let moduleFileNameForExtensionless: string | undefined;
while (true) {
// If the module could be imported by a directory name, use that directory's name
const { moduleFileToTry, packageRootPath } = tryDirectoryWithPackageJson(packageRootIndex);
if (packageRootPath) {
moduleSpecifier = packageRootPath;
break;
}
if (!moduleFileNameForExtensionless) moduleFileNameForExtensionless = moduleFileToTry;

// try with next level of directory
packageRootIndex = moduleFileName.indexOf(directorySeparator, packageRootIndex + 1);
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure I’m understanding this correctly, the idea is:

  1. we’ve located the moduleFileName, say, /project/node_modules/preact/hooks/src/index.js
  2. we look at the structure of the path (as before) to call /project/node_modules/preact the “package root”
  3. we see if moduleFileName is accessible (via a package.json main field) from /project/node_modules/preact
  4. if not, go to step 3 but with one more directory appended to the end (in the first iteration of this case, /project/node_modules/preact/hooks)

As contrasted to the previous behavior, where we’d only try the package root before giving up and using the full moduleFileName (optionally with /index.js taken off the end).

if (packageRootIndex === -1) {
moduleSpecifier = getExtensionlessFileName(moduleFileNameForExtensionless);
break;
}
}
}

// Simplify the full file path to something that can be resolved by Node.

// If the module could be imported by a directory name, use that directory's name
const moduleSpecifier = packageNameOnly ? moduleFileName : getDirectoryOrExtensionlessFileName(moduleFileName);
const globalTypingsCacheLocation = host.getGlobalTypingsCacheLocation && host.getGlobalTypingsCacheLocation();
// Get a path that's relative to node_modules or the importing file's path
// if node_modules folder is in this folder or any of its parent folders, no need to keep it.
Expand All @@ -360,18 +357,40 @@ namespace ts.moduleSpecifiers {
// For classic resolution, only allow importing from node_modules/@types, not other node_modules
return getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs && packageName === nodeModulesDirectoryName ? undefined : packageName;

function getDirectoryOrExtensionlessFileName(path: string): string {
// If the file is the main module, it can be imported by the package name
if (packageJsonContent) {
function tryDirectoryWithPackageJson(packageRootIndex: number) {
const packageRootPath = moduleFileName.substring(0, packageRootIndex);
const packageJsonPath = combinePaths(packageRootPath, "package.json");
let moduleFileToTry = moduleFileName;
if (host.fileExists(packageJsonPath)) {
const packageJsonContent = JSON.parse(host.readFile!(packageJsonPath)!);
const versionPaths = packageJsonContent.typesVersions
? getPackageJsonTypesVersionsPaths(packageJsonContent.typesVersions)
: undefined;
if (versionPaths) {
const subModuleName = moduleFileName.slice(packageRootPath.length + 1);
const fromPaths = tryGetModuleNameFromPaths(
removeFileExtension(subModuleName),
removeExtensionAndIndexPostFix(subModuleName, Ending.Minimal, options),
versionPaths.paths
);
if (fromPaths !== undefined) {
moduleFileToTry = combinePaths(packageRootPath, fromPaths);
}
}

// If the file is the main module, it can be imported by the package name
const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main;
if (isString(mainFileRelative)) {
const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName);
if (removeFileExtension(mainExportFile) === removeFileExtension(getCanonicalFileName(path))) {
return packageRootPath;
if (removeFileExtension(mainExportFile) === removeFileExtension(getCanonicalFileName(moduleFileToTry))) {
return { packageRootPath, moduleFileToTry };
}
}
}
return { moduleFileToTry };
}

function getExtensionlessFileName(path: string): string {
// We still have a file name - remove the extension
const fullModulePathWithoutExtension = removeFileExtension(path);

Expand Down
36 changes: 36 additions & 0 deletions tests/cases/fourslash/importFixesWithPackageJsonInSideAnotherPackage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/// <reference path="fourslash.ts" />

// @Filename: /project/tsconfig.json
////{
//// "compilerOptions": {
//// "jsx": "react",
//// "jsxFactory": "h"
//// }
////}

// @Filename: /project/app.tsx
////const state = useMemo(() => 'Hello', []);

// @Filename: /project/component.tsx
////import { useEffect } from "preact/hooks";

// @Filename: /project/node_modules/preact/package.json
////{ "name": "preact", "version": "10.3.4", "types": "src/index.d.ts" }

// @Filename: /project/node_modules/preact/hooks/package.json
////{ "name": "hooks", "version": "0.1.0", "types": "src/index.d.ts" }

// @Filename: /project/node_modules/preact/hooks/src/index.d.ts
////export function useEffect(): void;
////export function useMemo<T>(factory: () => T, inputs: ReadonlyArray<unknown> | undefined): T;

goTo.file("/project/app.tsx");
verify.importFixAtPosition([
getImportFixContent("preact/hooks"),
]);

function getImportFixContent(from: string) {
return `import { useMemo } from "${from}";

const state = useMemo(() => 'Hello', []);`;
}