Skip to content
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

[rush-lib] fix: update shrinkwrap when globalPackageExtensions has been changed #4913

Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Always update shrinkwrap when globalPackageExtensions has been changed",
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/node-core-library",
"comment": "Add Sort.sortKeys for sorting keys in an object",
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
"type": "minor"
}
],
"packageName": "@rushstack/node-core-library"
}
4 changes: 4 additions & 0 deletions common/reviews/api/node-core-library.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,10 @@ export class Sort {
static isSorted<T>(collection: Iterable<T>, comparer?: (x: any, y: any) => number): boolean;
static isSortedBy<T>(collection: Iterable<T>, keySelector: (element: T) => any, comparer?: (x: any, y: any) => number): boolean;
static sortBy<T>(array: T[], keySelector: (element: T) => any, comparer?: (x: any, y: any) => number): void;
static sortKeys<T extends Partial<Record<string, unknown>> | unknown[]>(object: T, { deep, compare }?: {
deep?: boolean;
compare?: (x: string, y: string) => number;
}): T;
static sortMapKeys<K, V>(map: Map<K, V>, keyComparer?: (x: K, y: K) => number): void;
static sortSet<T>(set: Set<T>, comparer?: (x: T, y: T) => number): void;
static sortSetBy<T>(set: Set<T>, keySelector: (element: T) => any, keyComparer?: (x: T, y: T) => number): void;
Expand Down
88 changes: 88 additions & 0 deletions libraries/node-core-library/src/Sort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,92 @@ export class Sort {
set.add(item);
}
}

/**
* Sort the keys given in an object
*
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
* @example
*
* ```ts
* console.log(Sort.sortKeys({ c: 3, b: 2, a: 1 })); // { a: 1, b: 2, c: 3}
* ```
*/
public static sortKeys<T extends Partial<Record<string, unknown>> | unknown[]>(
object: T,
{ deep, compare }: { deep?: boolean; compare?: (x: string, y: string) => number } = {
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
deep: false,
compare: Sort.compareByValue
}
): T {
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
function isPlainObject(obj: unknown): obj is object {
return obj !== null && typeof obj === 'object';
}
if (!isPlainObject(object) && !Array.isArray(object)) {
throw new TypeError(`Expected object or array`);
}

const cache: WeakMap<
Partial<Record<string, unknown>> | unknown[],
Partial<Record<string, unknown>> | unknown[]
> = new WeakMap();

function innerSortArray(arr: unknown[]): unknown[] {
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
const resultFromCache: undefined | Partial<Record<string, unknown>> | unknown[] = cache.get(arr);
if (resultFromCache !== undefined) {
return resultFromCache as unknown[];
}
const result: unknown[] = [];
cache.set(arr, result);
if (deep) {
result.push(
...arr.map((entry) => {
if (Array.isArray(entry)) {
return innerSortArray(entry);
} else if (isPlainObject(entry)) {
return innerSortKeys(entry);
}
return entry;
})
);
} else {
result.push(...arr);
}

return result;
}
function innerSortKeys(obj: Partial<Record<string, unknown>>): Partial<Record<string, unknown>> {
const resultFromCache: undefined | Partial<Record<string, unknown>> | unknown[] = cache.get(obj);
if (resultFromCache !== undefined) {
return resultFromCache as Partial<Record<string, unknown>>;
}
const result: Partial<Record<string, unknown>> = {};
const keys: string[] = Object.keys(obj).sort(compare);

cache.set(obj, result);

for (const key of keys) {
const value: unknown = obj[key];
let newValue: unknown;
if (deep) {
if (Array.isArray(value)) {
newValue = innerSortArray(value);
} else if (isPlainObject(value)) {
newValue = innerSortKeys(value);
} else {
newValue = value;
}
} else {
newValue = value;
}
Object.defineProperty(result, key, {
...Object.getOwnPropertyDescriptor(obj, key),
value: newValue
});
}

return result;
}

return Array.isArray(object) ? (innerSortArray(object) as T) : (innerSortKeys(object) as T);
}
}
179 changes: 179 additions & 0 deletions libraries/node-core-library/src/test/Sort.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,182 @@ test('Sort.sortSet', () => {
Sort.sortSet(set);
expect(Array.from(set)).toEqual(['aardvark', 'goose', 'zebra']);
});

describe('Sort.sortKeys', () => {
// Test cases from https://github.com/sindresorhus/sort-keys/blob/v4.2.0/test.js
function deepEqualInOrder(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
actual: Partial<Record<string, any>>,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
expected: Partial<Record<string, any>>
): void {
expect(actual).toEqual(expected);

const seen = new Set();

function assertSameKeysInOrder(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
object1: Partial<Record<string, any>>,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
object2: Partial<Record<string, any>>
): void {
// This function assumes the objects given are already deep equal.

if (seen.has(object1) && seen.has(object2)) {
return;
}

seen.add(object1);
seen.add(object2);

if (Array.isArray(object1)) {
for (const index of object1.keys()) {
assertSameKeysInOrder(object1[index], object2[index]);
}
} else if (typeof object1 === 'object') {
const keys1 = Object.keys(object1);
const keys2 = Object.keys(object2);
expect(keys1).toEqual(keys2);
for (const index of keys1.keys()) {
assertSameKeysInOrder(object1[keys1[index]], object2[keys2[index]]);
}
}
}

assertSameKeysInOrder(actual, expected);
}

test('sort the keys of an object', () => {
deepEqualInOrder(Sort.sortKeys({ c: 0, a: 0, b: 0 }), { a: 0, b: 0, c: 0 });
});

test('custom compare function', () => {
const compare: (a: string, b: string) => number = (a: string, b: string) => b.localeCompare(a);
deepEqualInOrder(Sort.sortKeys({ c: 0, a: 0, b: 0 }, { compare }), { c: 0, b: 0, a: 0 });
});

test('deep option', () => {
deepEqualInOrder(Sort.sortKeys({ c: { c: 0, a: 0, b: 0 }, a: 0, b: 0 }, { deep: true }), {
a: 0,
b: 0,
c: { a: 0, b: 0, c: 0 }
});

expect(() => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object: Partial<Record<string, any>> = { a: 0 };
object.circular = object;
Sort.sortKeys(object, { deep: true });
}).not.toThrow();
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object: Partial<Record<string, any>> = { z: 0 };
object.circular = object;
const sortedObject = Sort.sortKeys(object, { deep: true });

expect(sortedObject).toBe(sortedObject.circular);
expect(Object.keys(sortedObject)).toEqual(['circular', 'z']);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object1: Partial<Record<string, any>> = { b: 0 };
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object2: Partial<Record<string, any>> = { d: 0 };
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object3: Partial<Record<string, any>> = { a: [{ b: 0 }] };
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object4: Partial<Record<string, any>> = { a: [{ d: 0 }] };

object1.a = object2;
object2.c = object1;
object3.a[0].a = object4.a[0];
object4.a[0].c = object3.a[0];

expect(() => {
Sort.sortKeys(object1, { deep: true });
Sort.sortKeys(object2, { deep: true });
Sort.sortKeys(object3, { deep: true });
Sort.sortKeys(object4, { deep: true });
}).not.toThrow();

const sorted = Sort.sortKeys(object1, { deep: true });
const deepSorted = Sort.sortKeys(object3, { deep: true });

expect(sorted).toBe(sorted.a.c);
deepEqualInOrder(deepSorted.a[0], deepSorted.a[0].a.c);
expect(Object.keys(sorted)).toStrictEqual(['a', 'b']);
expect(Object.keys(deepSorted.a[0])).toStrictEqual(['a', 'b']);
deepEqualInOrder(
Sort.sortKeys({ c: { c: 0, a: 0, b: 0 }, a: 0, b: 0, z: [9, 8, 7, 6, 5] }, { deep: true }),
{ a: 0, b: 0, c: { a: 0, b: 0, c: 0 }, z: [9, 8, 7, 6, 5] }
);
expect(Object.keys(Sort.sortKeys({ a: [{ b: 0, a: 0 }] }, { deep: true }).a[0])).toEqual(['a', 'b']);
});

test('deep arrays', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object: Partial<Record<string, any>> = {
b: 0,
a: [{ b: 0, a: 0 }, [{ b: 0, a: 0 }]]
};
object.a.push(object);
object.a[1].push(object.a[1]);

expect(() => {
Sort.sortKeys(object, { deep: true });
}).not.toThrow();

const sorted = Sort.sortKeys(object, { deep: true });
// Cannot use .toBe() as Jest will encounter https://github.com/jestjs/jest/issues/10577
expect(sorted.a[2] === sorted).toBeTruthy();
expect(sorted.a[1][1] === sorted.a[1]).toBeTruthy();
expect(Object.keys(sorted)).toEqual(['a', 'b']);
expect(Object.keys(sorted.a[0])).toEqual(['a', 'b']);
expect(Object.keys(sorted.a[1][0])).toEqual(['a', 'b']);
});

test('top-level array', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const array: Array<any> = [
{ b: 0, a: 0 },
{ c: 0, d: 0 }
];
const sorted = Sort.sortKeys(array);
expect(sorted).not.toBe(array);
expect(sorted[0]).toBe(array[0]);
expect(sorted[1]).toBe(array[1]);

const deepSorted = Sort.sortKeys(array, { deep: true });
expect(deepSorted).not.toBe(array);
expect(deepSorted[0]).not.toBe(array[0]);
expect(deepSorted[1]).not.toBe(array[1]);
expect(Object.keys(deepSorted[0])).toEqual(['a', 'b']);
expect(Object.keys(deepSorted[1])).toEqual(['c', 'd']);
});

test('keeps property descriptors intact', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const descriptors: Partial<Record<string, any>> = {
b: {
value: 1,
configurable: true,
enumerable: true,
writable: false
},
a: {
value: 2,
configurable: false,
enumerable: true,
writable: true
}
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const object: Partial<Record<string, any>> = {};
Object.defineProperties(object, descriptors);

const sorted = Sort.sortKeys(object);

deepEqualInOrder(sorted, { a: 2, b: 1 });
expect(Object.getOwnPropertyDescriptors(sorted)).toEqual(descriptors);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import {
AlreadyReportedError,
Async,
type IDependenciesMetaTable,
Path
Path,
Sort
} from '@rushstack/node-core-library';
import { createHash } from 'crypto';

import { BaseInstallManager } from '../base/BaseInstallManager';
import type { IInstallManagerOptions } from '../base/BaseInstallManagerTypes';
Expand Down Expand Up @@ -378,6 +380,18 @@ export class WorkspaceInstallManager extends BaseInstallManager {
shrinkwrapIsUpToDate = false;
}

// Check if packageExtensionsChecksum matches globalPackageExtension's hash
const packageExtensionsChecksum: string | undefined = this._getPackageExtensionChecksum(
this.rushConfiguration.pnpmOptions.globalPackageExtensions
);
const packageExtensionsChecksumAreEqual: boolean =
packageExtensionsChecksum === shrinkwrapFile?.packageExtensionsChecksum;

if (!packageExtensionsChecksumAreEqual) {
shrinkwrapWarnings.push("The package extension hash doesn't match the current shrinkwrap.");
shrinkwrapIsUpToDate = false;
}

// Write the common package.json
InstallHelpers.generateCommonPackageJson(this.rushConfiguration, subspace, undefined);

Expand All @@ -388,6 +402,22 @@ export class WorkspaceInstallManager extends BaseInstallManager {
return { shrinkwrapIsUpToDate, shrinkwrapWarnings };
}

private _getPackageExtensionChecksum(
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
packageExtensions: Record<string, unknown> | undefined
): string | undefined {
const packageExtensionsChecksum: string | undefined =
Object.keys(packageExtensions ?? {}).length === 0
? undefined
: createObjectChecksum(packageExtensions!);

function createObjectChecksum(obj: Record<string, unknown>): string {
kenrick95 marked this conversation as resolved.
Show resolved Hide resolved
const s: string = JSON.stringify(Sort.sortKeys(obj, { deep: true }));
iclanton marked this conversation as resolved.
Show resolved Hide resolved
return createHash('md5').update(s).digest('hex');
}

return packageExtensionsChecksum;
}

protected canSkipInstall(lastModifiedDate: Date, subspace: Subspace): boolean {
if (!super.canSkipInstall(lastModifiedDate, subspace)) {
return false;
Expand Down
4 changes: 4 additions & 0 deletions libraries/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ export interface IPnpmShrinkwrapYaml {
specifiers: Record<string, string>;
/** The list of override version number for dependencies */
overrides?: { [dependency: string]: string };
/** The checksum of package extensions fields for extending dependencies */
packageExtensionsChecksum?: string;
}

export interface ILoadFromFileOptions {
Expand Down Expand Up @@ -275,6 +277,7 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
public readonly specifiers: ReadonlyMap<string, string>;
public readonly packages: ReadonlyMap<string, IPnpmShrinkwrapDependencyYaml>;
public readonly overrides: ReadonlyMap<string, string>;
public readonly packageExtensionsChecksum: undefined | string;

private readonly _shrinkwrapJson: IPnpmShrinkwrapYaml;
private readonly _integrities: Map<string, Map<string, string>>;
Expand Down Expand Up @@ -304,6 +307,7 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
this.specifiers = new Map(Object.entries(shrinkwrapJson.specifiers || {}));
this.packages = new Map(Object.entries(shrinkwrapJson.packages || {}));
this.overrides = new Map(Object.entries(shrinkwrapJson.overrides || {}));
this.packageExtensionsChecksum = shrinkwrapJson.packageExtensionsChecksum;

// Importers only exist in workspaces
this.isWorkspaceCompatible = this.importers.size > 0;
Expand Down
Loading