Skip to content

Commit 39b8609

Browse files
ecraig12345ecraig12345
committed
Fix logic to error on duplicate package names
1 parent 0db5f35 commit 39b8609

File tree

3 files changed

+287
-7
lines changed

3 files changed

+287
-7
lines changed

‎change/beachball-0b973948-5e88-4403-9731-6fbb9b375e4a.json ‎change/beachball-7e671837-50e2-4312-922e-36dcbd734803.json

File renamed without changes.

‎src/__e2e__/monorepo/getPackageInfos.test.ts

+275
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
import fs from 'fs-extra';
2+
import { RepositoryFactory } from '../../__fixtures__/repositoryFactory';
3+
import { tmpdir } from '../../__fixtures__/tmpdir';
4+
import { getPackageInfos } from '../../monorepo/getPackageInfos';
5+
import { PackageInfos } from '../../types/PackageInfo';
6+
import { getDefaultOptions } from '../../options/getDefaultOptions';
7+
import { getCliOptions } from '../../options/getCliOptions';
8+
import { gitFailFast } from 'workspace-tools';
9+
10+
const defaultOptions = getDefaultOptions();
11+
const cliOptions = getCliOptions(process.argv);
12+
13+
/** Strip the root path from the file path and normalize slashes */
14+
function cleanPath(root: string, filePath: string) {
15+
root = root.replace(/\\/g, '/');
16+
return filePath.replace(/\\/g, '/').replace(root, '').slice(1);
17+
}
18+
19+
/** Strip unneeded info from the result of `getPackageInfos` before taking snapshots */
20+
function cleanPackageInfos(root: string, packageInfos: PackageInfos) {
21+
const cleanedInfos: PackageInfos = {};
22+
for (const [pkgName, originalInfo] of Object.entries(packageInfos)) {
23+
// Make a copy deep enough to cover anything that will be modified
24+
const pkgInfo = (cleanedInfos[pkgName] = { ...originalInfo, combinedOptions: { ...originalInfo.combinedOptions } });
25+
26+
// Remove absolute paths
27+
pkgInfo.packageJsonPath = cleanPath(root, pkgInfo.packageJsonPath);
28+
(pkgInfo.combinedOptions as any).path = cleanPath(root, (pkgInfo.combinedOptions as any).path);
29+
30+
// Remove beachball options which are defaulted or not useful.
31+
// Also remove jest CLI or config options which get mixed in...
32+
for (const [key, value] of Object.entries(pkgInfo.combinedOptions)) {
33+
if (
34+
(defaultOptions as any)[key] === value ||
35+
(cliOptions as any)[key] ||
36+
key.startsWith('test') ||
37+
['roots', 'transform'].includes(key)
38+
) {
39+
delete (pkgInfo.combinedOptions as any)[key];
40+
}
41+
}
42+
43+
// Remove options set to undefined or empty object (keep null because it may be meaningful/interesting)
44+
for (const [key, value] of Object.entries(pkgInfo)) {
45+
if (value === undefined || (value && typeof value === 'object' && !Object.keys(value).length)) {
46+
delete (pkgInfo as any)[key];
47+
}
48+
}
49+
}
50+
51+
return cleanedInfos;
52+
}
53+
54+
/** Return an object mapping package names to package.json paths */
55+
function getPackageNamesAndPaths(root: string, packageInfos: PackageInfos) {
56+
return Object.fromEntries(
57+
Object.entries(packageInfos).map(([name, pkg]) => [name, cleanPath(root, pkg.packageJsonPath)])
58+
);
59+
}
60+
61+
describe('getPackageInfos', () => {
62+
// factories can be reused between these tests because none of them push changes
63+
let singleFactory: RepositoryFactory;
64+
let monorepoFactory: RepositoryFactory;
65+
let multiWorkspaceFactory: RepositoryFactory;
66+
let tempDir: string | undefined;
67+
68+
beforeAll(() => {
69+
singleFactory = new RepositoryFactory('single');
70+
monorepoFactory = new RepositoryFactory('monorepo');
71+
multiWorkspaceFactory = new RepositoryFactory('multi-workspace');
72+
});
73+
74+
afterEach(() => {
75+
tempDir && fs.removeSync(tempDir);
76+
tempDir = undefined;
77+
});
78+
79+
afterAll(() => {
80+
singleFactory.cleanUp();
81+
monorepoFactory.cleanUp();
82+
multiWorkspaceFactory.cleanUp();
83+
});
84+
85+
it('throws if run outside a git repo', () => {
86+
tempDir = tmpdir();
87+
expect(() => getPackageInfos(tempDir!)).toThrow(/not in a git repository/);
88+
});
89+
90+
it('returns empty object if no packages are found', () => {
91+
tempDir = tmpdir();
92+
gitFailFast(['init'], { cwd: tempDir });
93+
expect(getPackageInfos(tempDir)).toEqual({});
94+
});
95+
96+
it('works in single-package repo', () => {
97+
const repo = singleFactory.cloneRepository();
98+
let packageInfos = getPackageInfos(repo.rootPath);
99+
packageInfos = cleanPackageInfos(repo.rootPath, packageInfos);
100+
expect(packageInfos).toMatchInlineSnapshot(`
101+
Object {
102+
"foo": Object {
103+
"dependencies": Object {
104+
"bar": "1.0.0",
105+
"baz": "1.0.0",
106+
},
107+
"name": "foo",
108+
"packageJsonPath": "package.json",
109+
"private": false,
110+
"version": "1.0.0",
111+
},
112+
}
113+
`);
114+
});
115+
116+
// both yarn and npm define "workspaces" in package.json
117+
it('works in yarn/npm monorepo', () => {
118+
const repo = monorepoFactory.cloneRepository();
119+
let packageInfos = getPackageInfos(repo.rootPath);
120+
packageInfos = cleanPackageInfos(repo.rootPath, packageInfos);
121+
expect(packageInfos).toMatchInlineSnapshot(`
122+
Object {
123+
"a": Object {
124+
"name": "a",
125+
"packageJsonPath": "packages/grouped/a/package.json",
126+
"private": false,
127+
"version": "3.1.2",
128+
},
129+
"b": Object {
130+
"name": "b",
131+
"packageJsonPath": "packages/grouped/b/package.json",
132+
"private": false,
133+
"version": "3.1.2",
134+
},
135+
"bar": Object {
136+
"dependencies": Object {
137+
"baz": "^1.3.4",
138+
},
139+
"name": "bar",
140+
"packageJsonPath": "packages/bar/package.json",
141+
"private": false,
142+
"version": "1.3.4",
143+
},
144+
"baz": Object {
145+
"name": "baz",
146+
"packageJsonPath": "packages/baz/package.json",
147+
"private": false,
148+
"version": "1.3.4",
149+
},
150+
"foo": Object {
151+
"dependencies": Object {
152+
"bar": "^1.3.4",
153+
},
154+
"name": "foo",
155+
"packageJsonPath": "packages/foo/package.json",
156+
"private": false,
157+
"version": "1.0.0",
158+
},
159+
}
160+
`);
161+
});
162+
163+
it('works in pnpm monorepo', () => {
164+
const repo = monorepoFactory.cloneRepository();
165+
fs.writeJSONSync(repo.pathTo('package.json'), { name: 'pnpm-monorepo', version: '1.0.0', private: true });
166+
fs.writeFileSync(repo.pathTo('pnpm-lock.yaml'), '');
167+
fs.writeFileSync(repo.pathTo('pnpm-workspace.yaml'), 'packages: ["packages/*", "packages/grouped/*"]');
168+
169+
const rootPackageInfos = getPackageInfos(repo.rootPath);
170+
expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
171+
Object {
172+
"a": "packages/grouped/a/package.json",
173+
"b": "packages/grouped/b/package.json",
174+
"bar": "packages/bar/package.json",
175+
"baz": "packages/baz/package.json",
176+
"foo": "packages/foo/package.json",
177+
"pnpm-monorepo": "package.json",
178+
}
179+
`);
180+
});
181+
182+
it('works in rush monorepo', () => {
183+
const repo = monorepoFactory.cloneRepository();
184+
fs.writeJSONSync(repo.pathTo('package.json'), { name: 'rush-monorepo', version: '1.0.0', private: true });
185+
fs.writeJSONSync(repo.pathTo('rush.json'), {
186+
projects: [{ projectFolder: 'packages' }, { projectFolder: 'packages/grouped' }],
187+
});
188+
189+
const rootPackageInfos = getPackageInfos(repo.rootPath);
190+
expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
191+
Object {
192+
"a": "packages/grouped/a/package.json",
193+
"b": "packages/grouped/b/package.json",
194+
"bar": "packages/bar/package.json",
195+
"baz": "packages/baz/package.json",
196+
"foo": "packages/foo/package.json",
197+
"rush-monorepo": "package.json",
198+
}
199+
`);
200+
});
201+
202+
it('works in lerna monorepo', () => {
203+
const repo = monorepoFactory.cloneRepository();
204+
fs.writeJSONSync(repo.pathTo('package.json'), { name: 'lerna-monorepo', version: '1.0.0', private: true });
205+
fs.writeJSONSync(repo.pathTo('lerna.json'), { packages: ['packages/*', 'packages/grouped/*'] });
206+
207+
const rootPackageInfos = getPackageInfos(repo.rootPath);
208+
expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
209+
Object {
210+
"a": "packages/grouped/a/package.json",
211+
"b": "packages/grouped/b/package.json",
212+
"bar": "packages/bar/package.json",
213+
"baz": "packages/baz/package.json",
214+
"foo": "packages/foo/package.json",
215+
}
216+
`);
217+
});
218+
219+
it('works multi-workspace monorepo', () => {
220+
const repo = multiWorkspaceFactory.cloneRepository();
221+
222+
// For this test, only snapshot the package names and paths
223+
const rootPackageInfos = getPackageInfos(repo.rootPath);
224+
expect(getPackageNamesAndPaths(repo.rootPath, rootPackageInfos)).toMatchInlineSnapshot(`
225+
Object {
226+
"@workspace-a/a": "workspace-a/packages/grouped/a/package.json",
227+
"@workspace-a/b": "workspace-a/packages/grouped/b/package.json",
228+
"@workspace-a/bar": "workspace-a/packages/bar/package.json",
229+
"@workspace-a/baz": "workspace-a/packages/baz/package.json",
230+
"@workspace-a/foo": "workspace-a/packages/foo/package.json",
231+
"@workspace-a/monorepo-fixture": "workspace-a/package.json",
232+
"@workspace-b/a": "workspace-b/packages/grouped/a/package.json",
233+
"@workspace-b/b": "workspace-b/packages/grouped/b/package.json",
234+
"@workspace-b/bar": "workspace-b/packages/bar/package.json",
235+
"@workspace-b/baz": "workspace-b/packages/baz/package.json",
236+
"@workspace-b/foo": "workspace-b/packages/foo/package.json",
237+
"@workspace-b/monorepo-fixture": "workspace-b/package.json",
238+
}
239+
`);
240+
241+
const workspaceARoot = repo.pathTo('workspace-a');
242+
const packageInfosA = getPackageInfos(workspaceARoot);
243+
expect(getPackageNamesAndPaths(workspaceARoot, packageInfosA)).toMatchInlineSnapshot(`
244+
Object {
245+
"@workspace-a/a": "packages/grouped/a/package.json",
246+
"@workspace-a/b": "packages/grouped/b/package.json",
247+
"@workspace-a/bar": "packages/bar/package.json",
248+
"@workspace-a/baz": "packages/baz/package.json",
249+
"@workspace-a/foo": "packages/foo/package.json",
250+
}
251+
`);
252+
253+
const workspaceBRoot = repo.pathTo('workspace-b');
254+
const packageInfosB = getPackageInfos(workspaceBRoot);
255+
expect(getPackageNamesAndPaths(workspaceBRoot, packageInfosB)).toMatchInlineSnapshot(`
256+
Object {
257+
"@workspace-b/a": "packages/grouped/a/package.json",
258+
"@workspace-b/b": "packages/grouped/b/package.json",
259+
"@workspace-b/bar": "packages/bar/package.json",
260+
"@workspace-b/baz": "packages/baz/package.json",
261+
"@workspace-b/foo": "packages/foo/package.json",
262+
}
263+
`);
264+
});
265+
266+
it('throws if multiple packages have the same name in multi-workspace monorepo', () => {
267+
// If there are multiple workspaces in a monorepo, it's possible that two packages in different
268+
// workspaces could share the same name, which causes problems for beachball.
269+
// (This is only known to have been an issue with the test fixture, but is worth testing.)
270+
const repo = multiWorkspaceFactory.cloneRepository();
271+
repo.updateJsonFile('workspace-a/packages/foo/package.json', { name: 'foo' });
272+
repo.updateJsonFile('workspace-b/packages/foo/package.json', { name: 'foo' });
273+
expect(() => getPackageInfos(repo.rootPath)).toThrow();
274+
});
275+
});

‎src/monorepo/getPackageInfos.ts

+12-7
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,6 @@ function getPackageInfosFromWorkspace(projectRoot: string) {
3333
const packageJsonPath = path.join(packagePath, 'package.json');
3434

3535
try {
36-
if (packageInfos[packageJson.name]) {
37-
throw new Error(
38-
`Two packages in different workspaces have the same name. Please rename one of these packages:\n- ${
39-
packageInfos[packageJson.name].packageJsonPath
40-
}\n- ${packageJsonPath}`
41-
);
42-
}
4336
packageInfos[packageJson.name] = infoFromPackageJson(packageJson, packageJsonPath);
4437
} catch (e) {
4538
// Pass, the package.json is invalid
@@ -61,11 +54,23 @@ function getPackageInfosFromNonWorkspaceMonorepo(projectRoot: string) {
6154

6255
if (packageJsonFiles && packageJsonFiles.length > 0) {
6356
packageJsonFiles.forEach(packageJsonPath => {
57+
let hasDuplicatePackage = false;
6458
try {
6559
const packageJsonFullPath = path.join(projectRoot, packageJsonPath);
6660
const packageJson = fs.readJSONSync(packageJsonFullPath);
61+
if (packageInfos[packageJson.name]) {
62+
hasDuplicatePackage = true;
63+
throw new Error(
64+
`Two packages in different workspaces have the same name. Please rename one of these packages:\n- ${
65+
packageInfos[packageJson.name].packageJsonPath
66+
}\n- ${packageJsonPath}`
67+
);
68+
}
6769
packageInfos[packageJson.name] = infoFromPackageJson(packageJson, packageJsonFullPath);
6870
} catch (e) {
71+
if (hasDuplicatePackage) {
72+
throw e; // duplicate package error should propagate
73+
}
6974
// Pass, the package.json is invalid
7075
console.warn(`Invalid package.json file detected ${packageJsonPath}: `, e);
7176
}

0 commit comments

Comments
 (0)