Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

fix path mapping checks for scoped and versioned packages #507

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix path mapping checks for scoped and versioned packages #507

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
11 changes: 7 additions & 4 deletions src/lib/definition-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as ts from "typescript";

import { FS } from "../get-definitely-typed";
import {
computeHash, filter, flatMap, hasWindowsSlashes, join, mapAsyncOrdered, mapDefined, split, unique, unmangleScopedPackage, withoutStart,
computeHash, filter, flatMap, hasWindowsSlashes, join, mapAsyncOrdered, mapDefined, split, stripVersion, unique, unmangleScopedPackage, withoutStart,
} from "../util/util";

import getModuleInfo, { getTestDependencies } from "./module-info";
Expand Down Expand Up @@ -282,8 +282,8 @@ async function calculateDependencies(
}
const pathMapping = pathMappingList[0];

// Path mapping may be for "@foo/bar" -> "foo__bar".
const scopedPackageName = unmangleScopedPackage(pathMapping);
// Path mapping may be for "@foo/bar" -> "foo__bar" or "@foo/bar" -> "foo__bar/v2"
const scopedPackageName = unmangleScopedPackage(stripVersion(pathMapping));
if (scopedPackageName !== undefined) {
if (dependencyName !== scopedPackageName) {
throw new Error(`Expected directory ${pathMapping} to be the path mapping for ${dependencyName}`);
Copy link
Member

Choose a reason for hiding this comment

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

calculateDependencies is odd in that only versioned packages need an entry in the dependencies it calculates. Previously, since we didn't check scoped packages for versions, the continue below was fine.

However, if you want to correctly handle versioned, scoped packages, then you'll have to move this section below the version-checking code that starts with const majorVersion = ... on line 294. Then that section will need to handle scoped packages correctly. I guess that the handling will be something like { name: scopedPackageName, majorVersion }, where scopedPackageName = unmangleScopedPackage(stripVersion(pathMapping)) (like you have above) and majorVersion = parseDependencyVersionFromPath(.... But you'll have to make sure that parseDependencyVersionFromPath handles scoped packages as well, perhaps by calling it with different arguments for the scoped version case.

Expand All @@ -308,7 +308,10 @@ async function calculateDependencies(
pathMappings.push({ packageName: dependencyName, majorVersion });
}

if (oldMajorVersion !== undefined && !(paths && packageName in paths)) {
// Path mapping may be for "foo" -> "foo/v2" or "@foo/bar" -> "foo__bar/v2"
const scopedName = unmangleScopedPackage(packageName);
const hasSelfPathMapping = scopedName !== undefined ? scopedName in paths : packageName in paths;
if (oldMajorVersion !== undefined && !hasSelfPathMapping) {
throw new Error(`${packageName}: Older version ${oldMajorVersion} must have a path mapping for itself.`);
}

Expand Down
5 changes: 5 additions & 0 deletions src/util/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,11 @@ export function unmangleScopedPackage(packageName: string): string | undefined {
return packageName.includes(separator) ? `@${packageName.replace(separator, "/")}` : undefined;
}

const versionAtEndRegex = /\/v\d+$/;
export function stripVersion(packageName: string): string {
return packageName.replace(versionAtEndRegex, "");
}

/** Returns [values that cb returned undefined for, defined results of cb]. */
export function split<T, U>(inputs: ReadonlyArray<T>, cb: (t: T) => U | undefined): [ReadonlyArray<T>, ReadonlyArray<U>] {
const keep: T[] = [];
Expand Down