Skip to content

Commit

Permalink
Merge pull request #3108 from relative-ci/revert-insights-changed
Browse files Browse the repository at this point in the history
Revert "feat(utils): Utils - add insight changes flag"
  • Loading branch information
vio committed Feb 11, 2023
2 parents b9c27a5 + a99c9cd commit b0744a5
Show file tree
Hide file tree
Showing 9 changed files with 7 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ exports[`Storyshots App default 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"d3-time": Array [
Expand All @@ -73,7 +72,6 @@ exports[`Storyshots App default 1`] = `
"type": "error",
},
"newPackages": Object {
"changes": true,
"data": Object {
"packages": Array [
"ariakit-utils",
Expand Down Expand Up @@ -29795,7 +29793,6 @@ exports[`Storyshots App default 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"hoist-non-react-statics": Array [
Expand Down Expand Up @@ -60035,7 +60032,6 @@ exports[`Storyshots App empty baseline 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"d3-time": Array [
Expand Down Expand Up @@ -89780,7 +89776,6 @@ exports[`Storyshots App multiple baselines 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"d3-time": Array [
Expand All @@ -89803,7 +89798,6 @@ exports[`Storyshots App multiple baselines 1`] = `
"type": "error",
},
"newPackages": Object {
"changes": true,
"data": Object {
"packages": Array [
"ariakit-utils",
Expand Down Expand Up @@ -119592,7 +119586,6 @@ exports[`Storyshots App multiple baselines 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"hoist-non-react-statics": Array [
Expand All @@ -119619,7 +119612,6 @@ exports[`Storyshots App multiple baselines 1`] = `
"type": "error",
},
"newPackages": Object {
"changes": true,
"data": Object {
"packages": Array [
"tiny-invariant",
Expand Down Expand Up @@ -149886,7 +149878,6 @@ exports[`Storyshots App multiple baselines 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"tslib": Array [
Expand Down Expand Up @@ -153027,7 +153018,6 @@ exports[`Storyshots App no baseline 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"d3-time": Array [
Expand All @@ -153050,7 +153040,6 @@ exports[`Storyshots App no baseline 1`] = `
"type": "error",
},
"newPackages": Object {
"changes": true,
"data": Object {
"packages": Array [
"ariakit-utils",
Expand Down Expand Up @@ -212454,7 +212443,6 @@ exports[`Storyshots App no insights 1`] = `
"type": "warning",
},
"duplicatePackagesV3": Object {
"changes": true,
"data": Object {
"packages": Object {
"hoist-non-react-statics": Array [
Expand Down
3 changes: 0 additions & 3 deletions packages/ui/src/components/insights/insights.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ Default.args = {
text: 'Bundle introduced 2 new packages: package-c, package-d',
packages: ['package-c', 'package-d'],
},
changes: true,
},
};

Expand All @@ -40,14 +39,12 @@ Info.args = {
packages: { 'package-a': ['package-a', 'package-a~1'] },
text: 'Bundle removed 1 duplicate packages',
},
changes: false,
},
newPackages: {
type: 'warning',
data: {
text: 'Bundle introduced 2 new packages: package-c, package-d',
packages: ['package-c', 'package-d'],
},
changes: true,
},
};
10 changes: 0 additions & 10 deletions packages/utils/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,8 @@ export interface JobSummaryItem {
}

export interface JobInsight<T = object> {
/**
* Insight type
*/
type: InsightType;
/**
* Insight result data
*/
data: T & { text?: string };
/**
* Flag if the insight result is due to changes between jobs
*/
changes?: boolean;
}

export interface JobInsightAssetsSizeTotalData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ exports[`createJob no baseline 1`] = `
"type": "warning",
},
"duplicatePackagesV3": {
"changes": true,
"data": {
"packages": {
"package-c": [
Expand Down Expand Up @@ -408,7 +407,6 @@ exports[`createJob with baseline 1`] = `
"type": "warning",
},
"duplicatePackagesV3": {
"changes": true,
"data": {
"packages": {
"package-c": [
Expand All @@ -421,7 +419,6 @@ exports[`createJob with baseline 1`] = `
"type": "error",
},
"newPackages": {
"changes": true,
"data": {
"packages": [
"package-d",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ exports[`report / createReport multiple sources 1`] = `
"type": "warning",
},
"duplicatePackagesV3": {
"changes": true,
"data": {
"packages": {
"package-c": [
Expand All @@ -166,7 +165,6 @@ exports[`report / createReport multiple sources 1`] = `
"type": "error",
},
"newPackages": {
"changes": true,
"data": {
"packages": [
"package-d",
Expand Down Expand Up @@ -1123,7 +1121,6 @@ exports[`report / createReport single source 1`] = `
"type": "warning",
},
"duplicatePackagesV3": {
"changes": true,
"data": {
"packages": {
"package-c": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
packages: {},
text: 'Bundle does not contain duplicate packages',
},
changes: false,
});
expect(getDuplicatePackagesInsight({}, {})).toEqual({
type: 'info',
data: {
packages: {},
text: 'Bundle does not contain duplicate packages',
},
changes: false,
});
});

Expand All @@ -39,7 +37,6 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
},
text: 'Bundle contains 2 duplicate packages',
},
changes: false,
});
});

Expand All @@ -64,7 +61,6 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
},
text: 'Bundle contains 2 duplicate packages',
},
changes: false,
});
});

Expand All @@ -89,7 +85,6 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
},
text: 'Bundle introduced 1 duplicate package',
},
changes: true,
});
});

Expand All @@ -109,7 +104,6 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
packages: { 'package-a': ['package-a', 'package-a~1'] },
text: 'Bundle introduced 1 and removed 1 duplicate package',
},
changes: true,
});
});
});
Expand All @@ -127,7 +121,6 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
},
},
metrics: { duplicatePackagesCount: { value: 0 } },
changes: false,
});
});

Expand Down Expand Up @@ -181,7 +174,6 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
'package-a': ['package-a', 'package-b:package-a'],
},
},
changes: true,
},
},
metrics: {
Expand Down Expand Up @@ -258,7 +250,6 @@ describe('Webpack/extract/extractModulesPackagesDuplicate', () => {
'package-a': ['package-a', 'package-b:package-a'],
},
},
changes: true,
},
},
metrics: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ describe('Webpack/extracts/extractModulesPackagesNewInsight', () => {
text: 'Bundle introduced one new package: package-b',
packages: ['package-b'],
},
changes: true,
},
},
});
Expand Down Expand Up @@ -154,7 +153,6 @@ describe('Webpack/extracts/extractModulesPackagesNewInsight', () => {
text: 'Bundle introduced 2 new packages: package-c, package-d',
packages: ['package-c', 'package-d'],
},
changes: true,
},
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,18 @@ export const getDuplicatePackagesInsight = (

let text: string;
let insightType: InsightType;
let changes = false;

if (newDuplicateInstancesCount > 0 && removedDuplicateInstancesCount > 0) {
// New duplicate packages and removed duplicates
const item =
newDuplicateInstancesCount > 1 || removedDuplicateInstancesCount > 1 ? 'packages' : 'package';
text = `Bundle introduced ${newDuplicateInstancesCount} and removed ${removedDuplicateInstancesCount} duplicate ${item}`;
insightType = InsightType.ERROR;
changes = true;
} else if (newDuplicateInstancesCount > 0 && baselineDuplicatePackagesMap) {
// New duplicate packages and baseline comparison
const item = newDuplicateInstancesCount > 1 ? 'packages' : 'package';
text = `Bundle introduced ${newDuplicateInstancesCount} duplicate ${item}`;
insightType = InsightType.ERROR;
changes = true;
} else if (newDuplicateInstancesCount > 0) {
// New duplicate packages and no baseline
const item = newDuplicateInstancesCount > 1 ? 'packages' : 'package';
Expand All @@ -108,7 +105,6 @@ export const getDuplicatePackagesInsight = (
const item = removedDuplicateInstancesCount > 1 ? 'packages' : 'package';
text = `Bundle removed ${removedDuplicateInstancesCount} duplicate ${item}`;
insightType = InsightType.INFO;
changes = true;
} else if (
newDuplicateInstancesCount === 0 &&
removedDuplicateInstancesCount === 0 &&
Expand All @@ -130,7 +126,6 @@ export const getDuplicatePackagesInsight = (
text,
packages: duplicatePackagesMap,
},
changes
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ const getText = (newPackages: Array<string>) => {

const text = `Bundle introduced ${newPackages.length} new packages: ${firstNewPackages.join(', ')}`;

const otherPackagesCount = otherPackages.length;

if (otherPackagesCount === 0) {
if (otherPackages.length === 0) {
return text;
}

return `${text} and ${otherPackagesCount > 1 ? otherPackagesCount : 'one'} more`;
if (otherPackages.length === 1) {
return `${text} and one more`;
}

return `${text} and ${otherPackages.length} more`;
};

export const extractModulesPackagesNewInsight = (
Expand All @@ -52,8 +54,8 @@ export const extractModulesPackagesNewInsight = (
const currentPackages = getPackageNames(currentMetricPackages);
const baselinePackages = getPackageNames(baselineMetricPackages);

// Process current packages and save if they are not available on the baseline job
const newPackages: Array<string> = [];

currentPackages.forEach((packageName) => {
if (!baselinePackages.has(packageName)) {
newPackages.push(packageName);
Expand All @@ -72,7 +74,6 @@ export const extractModulesPackagesNewInsight = (
text: getText(newPackages),
packages: newPackages,
},
changes: true,
},
},
};
Expand Down

0 comments on commit b0744a5

Please sign in to comment.