miel.truyen/nx
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

fix(core): fix json diff and implicitJsonChanges

Browse Source
This commit is contained in:
Jason Jean 2020-03-05 20:51:16 -05:00 committed by Victor Savkin
parent fef54fae4b
commit dbf6fd525d
9 changed files with 267 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/workspace/src/core/affected-project-graph/locators/implicit-json-changes.ts
View File

@ -65,5 +65,5 @@ function getTouchedProjects(path: string[], implicitDependencyConfig: any) {
break;
}
}
return found ? curr : [];
return found && Array.isArray(curr) ? curr : [];
}

31
packages/workspace/src/core/affected-project-graph/locators/npm-packages.spec.ts
View File

@ -90,6 +90,37 @@ describe('getTouchedNpmPackages', () => {
expect(result).toEqual(['proj1', 'proj2']);
});
it('should handle package addition', () => {
const result = getTouchedNpmPackages(
[
{
file: 'package.json',
mtime: 0,
ext: '.json',
getChanges: () => [
{
type: DiffType.Added,
path: ['dependencies', 'awesome-nrwl'],
value: {
lhs: undefined,
rhs: '0.0.1'
}
}
]
}
],
workspaceJson,
nxJson,
{
dependencies: {
'happy-nrwl': '0.0.2',
'awesome-nrwl': '0.0.1'
}
}
);
expect(result).toEqual(['awesome-nrwl']);
});
it('should handle whole file changes', () => {
const result = getTouchedNpmPackages(
[

3
packages/workspace/src/core/affected-project-graph/locators/npm-packages.ts
View File

@ -14,7 +14,8 @@ export const getTouchedNpmPackages: TouchedProjectLocator<
for (const c of changes) {
if (
isJsonChange(c) &&
(c.path[0] === 'dependencies' || c.path[0] === 'devDependencies')
(c.path[0] === 'dependencies' || c.path[0] === 'devDependencies') &&
c.path.length === 2
) {
// A package was deleted so mark all workspace projects as touched.
if (c.type === DiffType.Deleted) {

36
packages/workspace/src/core/affected-project-graph/locators/nx-json-changes.spec.ts
View File

@ -95,6 +95,16 @@ describe('getTouchedProjectsInNxJson', () => {
ext: '.json',
mtime: 0,
getChanges: () => [
{
type: DiffType.Added,
path: ['projects', 'proj1'],
value: {
lhs: undefined,
rhs: {
tags: []
}
}
},
{
type: DiffType.Added,
path: ['projects', 'proj1', 'tags'],
@ -122,7 +132,7 @@ describe('getTouchedProjectsInNxJson', () => {
expect(result).toEqual(['proj1']);
});
it('should not return projects removed in nx.json', () => {
it('should return all projects when a project is removed', () => {
const result = getTouchedProjectsInNxJson(
[
{
@ -132,9 +142,11 @@ describe('getTouchedProjectsInNxJson', () => {
getChanges: () => [
{
type: DiffType.Deleted,
path: ['projects', 'proj3', 'tags'],
path: ['projects', 'proj3'],
value: {
lhs: [],
lhs: {
tags: []
},
rhs: undefined
}
}
@ -165,6 +177,24 @@ describe('getTouchedProjectsInNxJson', () => {
ext: '.json',
mtime: 0,
getChanges: () => [
{
type: DiffType.Modified,
path: ['projects', 'proj1'],
value: {
lhs: { tags: ['scope:feat'] },
rhs: {
tags: ['scope:shared']
}
}
},
{
type: DiffType.Modified,
path: ['projects', 'proj1', 'tags'],
value: {
lhs: ['scope:feat'],
rhs: ['scope:shared']
}
},
{
type: DiffType.Modified,
path: ['projects', 'proj1', 'tags', '0'],

22
packages/workspace/src/core/affected-project-graph/locators/nx-json-changes.ts
View File

@ -33,13 +33,21 @@ export const getTouchedProjectsInNxJson: TouchedProjectLocator<
continue;
}
if (nxJson.projects[change.path[1]]) {
touched.push(change.path[1]);
} else {
// The project was deleted so affect all projects
touched.push(...Object.keys(nxJson.projects));
// Break out of the loop after all projects have been added.
break;
// Only look for changes that are changes to the whole project definition
if (change.path.length !== 2) {
continue;
}
switch (change.type) {
case DiffType.Deleted: {
// We are not sure which projects used to depend on a deleted project
// so return all projects to be safe
return Object.keys(nxJson.projects);
}
default: {
// Add the project name
touched.push(change.path[1]);
}
}
}
return touched;

33
packages/workspace/src/core/affected-project-graph/locators/workspace-json-changes.spec.ts
View File

@ -95,10 +95,21 @@ describe('getTouchedProjectsInWorkspaceJson', () => {
getChanges: () => [
{
type: DiffType.Added,
path: ['projects', 'proj1', 'tags'],
path: ['projects', 'proj1'],
value: {
lhs: undefined,
rhs: []
rhs: {
root: 'proj1'
}
}
},
{
type: DiffType.Added,
path: ['projects', 'proj1', 'root'],
value: {
lhs: undefined,
rhs: 'proj1'
}
}
]
@ -125,9 +136,11 @@ describe('getTouchedProjectsInWorkspaceJson', () => {
getChanges: () => [
{
type: DiffType.Deleted,
path: ['projects', 'proj3', 'root'],
path: ['projects', 'proj3'],
value: {
lhs: 'proj3',
lhs: {
root: 'proj3'
},
rhs: undefined
}
}
@ -156,6 +169,18 @@ describe('getTouchedProjectsInWorkspaceJson', () => {
ext: '.json',
mtime: 0,
getChanges: () => [
{
type: DiffType.Modified,
path: ['projects', 'proj1'],
value: {
lhs: {
root: 'proj3'
},
rhs: {
root: 'proj1'
}
}
},
{
type: DiffType.Modified,
path: ['projects', 'proj1', 'root'],

24
packages/workspace/src/core/affected-project-graph/locators/workspace-json-changes.ts
View File

@ -3,7 +3,7 @@ import {
WholeFileChange,
workspaceFileName
} from '../../file-utils';
import { isJsonChange, JsonChange } from '../../../utils/json-diff';
import { DiffType, isJsonChange, JsonChange } from '../../../utils/json-diff';
import { TouchedProjectLocator } from '../affected-project-graph-models';
export const getTouchedProjectsInWorkspaceJson: TouchedProjectLocator<
@ -39,13 +39,21 @@ export const getTouchedProjectsInWorkspaceJson: TouchedProjectLocator<
continue;
}
if (workspaceJson.projects[change.path[1]]) {
touched.push(change.path[1]);
} else {
// The project was deleted so affect all projects
touched.push(...Object.keys(workspaceJson.projects));
// Break out of the loop after all projects have been added.
break;
// Only look for changes that are changes to the whole project definition
if (change.path.length !== 2) {
continue;
}
switch (change.type) {
case DiffType.Deleted: {
// We are not sure which projects used to depend on a deleted project
// so return all projects to be safe
return Object.keys(workspaceJson.projects);
}
default: {
// Add the project name
touched.push(change.path[1]);
}
}
}
return touched;

109
packages/workspace/src/utils/json-diff.spec.ts
View File

@ -1,7 +1,7 @@
import { jsonDiff, DiffType } from './json-diff';
describe('jsonDiff', () => {
it('should return deep diffs of two JSON objects', () => {
it('should return deep diffs of two JSON objects (including parents of children changes)', () => {
const result = jsonDiff(
{ x: 1, a: { b: { c: 1 } } },
{ y: 2, a: { b: { c: 2, d: 2 } } }
@ -9,6 +9,36 @@ describe('jsonDiff', () => {
expect(result).toEqual(
expect.arrayContaining([
{
type: DiffType.Modified,
path: ['a'],
value: {
lhs: {
b: {
c: 1
}
},
rhs: {
b: {
c: 2,
d: 2
}
}
}
},
{
type: DiffType.Modified,
path: ['a', 'b'],
value: {
lhs: {
c: 1
},
rhs: {
c: 2,
d: 2
}
}
},
{
type: DiffType.Modified,
path: ['a', 'b', 'c'],
@ -33,6 +63,83 @@ describe('jsonDiff', () => {
);
});
it('should have diffs for objects as well', () => {
const result = jsonDiff(
{
a: { b: 0 }
},
{
a: { b: 1 }
}
);
expect(result).toContainEqual({
type: DiffType.Modified,
path: ['a'],
value: {
lhs: {
b: 0
},
rhs: {
b: 1
}
}
});
expect(result).toContainEqual({
type: DiffType.Modified,
path: ['a', 'b'],
value: {
lhs: 0,
rhs: 1
}
});
const result2 = jsonDiff(
{},
{
a: {}
}
);
expect(result2).toContainEqual({
type: DiffType.Added,
path: ['a'],
value: { lhs: undefined, rhs: {} }
});
});
it('should work for added array items', () => {
const result = jsonDiff(
{
rules: ['rule1']
},
{
rules: ['rule1', 'rule2']
}
);
expect(result).toEqual(
expect.arrayContaining([
{
type: DiffType.Modified,
path: ['rules'],
value: {
lhs: ['rule1'],
rhs: ['rule1', 'rule2']
}
},
{
type: DiffType.Added,
path: ['rules', '1'],
value: {
lhs: undefined,
rhs: 'rule2'
}
}
])
);
});
it('should work well for package.json', () => {
const result = jsonDiff(
{

41
packages/workspace/src/utils/json-diff.ts
View File

@ -29,9 +29,6 @@ export function jsonDiff(lhs: any, rhs: any): JsonChange[] {
walkJsonTree(lhs, [], (path, lhsValue) => {
seenInLhs.add(hashArray(path));
if (typeof lhsValue === 'object') {
return true;
}
const rhsValue = getJsonValue(path, rhs);
if (rhsValue === undefined) {
result.push({
@ -42,7 +39,7 @@ export function jsonDiff(lhs: any, rhs: any): JsonChange[] {
rhs: undefined
}
});
} else if (lhsValue !== rhsValue) {
} else if (!deepEquals(lhsValue, rhsValue)) {
result.push({
type: DiffType.Modified,
path,
@ -52,13 +49,10 @@ export function jsonDiff(lhs: any, rhs: any): JsonChange[] {
}
});
}
return false;
return typeof lhsValue === 'object';
});
walkJsonTree(rhs, [], (path, rhsValue) => {
if (typeof rhsValue === 'object') {
return true;
}
const addedInRhs = !seenInLhs.has(hashArray(path));
if (addedInRhs) {
result.push({
@ -71,6 +65,7 @@ export function jsonDiff(lhs: any, rhs: any): JsonChange[] {
});
return false;
}
return typeof rhsValue === 'object';
});
return result;
@ -85,7 +80,6 @@ export function walkJsonTree(
if (!json || typeof json !== 'object') {
return;
}
Object.keys(json).forEach(key => {
const path = currPath.concat([key]);
const shouldContinue = visitor(path, json[key]);
@ -109,3 +103,32 @@ function getJsonValue(path: string[], json: any): void | any {
}
return curr;
}
function deepEquals(a: any, b: any): boolean {
if (a === b) {
return true;
}
// Values do not need to be checked for deep equality and the above is false
if (
// Values are different types
typeof a !== typeof b ||
// Values are the same type but not an object or array
(typeof a !== 'object' && !Array.isArray(a)) ||
// Objects are the same type, objects or arrays, but do not have the same number of keys
Object.keys(a).length !== Object.keys(b).length
) {
return false;
}
// Values need to be checked for deep equality
return Object.entries(a).reduce((equal, [key, aValue]) => {
// Skip other keys if it is already not equal.
if (!equal) {
return equal;
}
// Traverse the object
return deepEquals(aValue, b[key]);
}, true);
}