Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/popular-readers-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@changesets/cli": major
---

Fixed a regression that caused `changeset version` to fail on packages having a dev dependency on a skipped package.
5 changes: 5 additions & 0 deletions .changeset/smart-goats-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@changesets/get-dependents-graph": minor
---

Added a new `ignoreDevDependencies` option
3 changes: 2 additions & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"disableOptimisticBPs": true,
"windows": {
"program": "${workspaceFolder}/node_modules/jest/bin/jest"
}
},
"smartStep": false
}
]
}
35 changes: 35 additions & 0 deletions packages/cli/src/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,41 @@ describe("cli", () => {
expect(loggerErrorCalls.length).toEqual(0);
});

it("should not throw on a dev dependent on an unversioned private package", async () => {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

a similar scenario was already buggy before this recent regression, see:
#1361 (comment)

const cwd = await testdir({
"package.json": JSON.stringify({
private: true,
workspaces: ["packages/*"],
}),
"packages/pkg-a/package.json": JSON.stringify({
name: "pkg-a",
version: "1.0.0",
devDependencies: {
"pkg-b": "1.0.0",
},
}),
"packages/pkg-b/package.json": JSON.stringify({
name: "pkg-b",
version: "1.0.0",
private: true,
}),
".changeset/config.json": JSON.stringify({
privatePackages: {
tag: false,
version: false,
},
}),
});
try {
await run(["version"], {}, cwd);
} catch (e) {
// ignore the error. We just want to validate the error message
}

const loggerErrorCalls = (error as any).mock.calls;
expect(loggerErrorCalls.length).toEqual(0);
});

it("should throw if `--ignore` flag is used while ignore array is also defined in the config file ", async () => {
const cwd = await testdir({
"package.json": JSON.stringify({
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export async function run(

// validate that all dependents of skipped packages are also skipped
const dependentsGraph = getDependentsGraph(packages, {
ignoreDevDependencies: true,
bumpVersionsWithWorkspaceProtocolOnly:
config.bumpVersionsWithWorkspaceProtocolOnly,
});
Expand Down
26 changes: 19 additions & 7 deletions packages/get-dependents-graph/src/get-dependency-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ const DEPENDENCY_TYPES = [
"optionalDependencies",
] as const;

const getAllDependencies = (config: PackageJSON) => {
const getAllDependencies = (
config: PackageJSON,
ignoreDevDependencies: boolean
) => {
const allDependencies = new Map<string, string>();

for (const type of DEPENDENCY_TYPES) {
Expand All @@ -21,8 +24,10 @@ const getAllDependencies = (config: PackageJSON) => {
for (const name of Object.keys(deps)) {
const depRange = deps[name];
if (
(depRange.startsWith("link:") || depRange.startsWith("file:")) &&
type === "devDependencies"
type === "devDependencies" &&
(ignoreDevDependencies ||
depRange.startsWith("link:") ||
depRange.startsWith("file:"))
) {
continue;
}
Expand Down Expand Up @@ -50,9 +55,13 @@ const getValidRange = (potentialRange: string) => {

export default function getDependencyGraph(
packages: Packages,
opts?: {
{
ignoreDevDependencies = false,
bumpVersionsWithWorkspaceProtocolOnly = false,
}: {
ignoreDevDependencies?: boolean;
bumpVersionsWithWorkspaceProtocolOnly?: boolean;
}
} = {}
): {
graph: Map<string, { pkg: Package; dependencies: Array<string> }>;
valid: boolean;
Expand All @@ -77,7 +86,10 @@ export default function getDependencyGraph(
for (const pkg of queue) {
const { name } = pkg.packageJson;
const dependencies = [];
const allDependencies = getAllDependencies(pkg.packageJson);
const allDependencies = getAllDependencies(
pkg.packageJson,
ignoreDevDependencies
);

for (let [depName, depRange] of allDependencies) {
const match = packagesByName[depName];
Expand All @@ -93,7 +105,7 @@ export default function getDependencyGraph(
dependencies.push(depName);
continue;
}
} else if (opts?.bumpVersionsWithWorkspaceProtocolOnly === true) {
} else if (bumpVersionsWithWorkspaceProtocolOnly) {
continue;
}

Expand Down
10 changes: 5 additions & 5 deletions packages/get-dependents-graph/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import getDependencyGraph from "./get-dependency-graph";

export function getDependentsGraph(
packages: Packages,
opts?: { bumpVersionsWithWorkspaceProtocolOnly?: boolean }
opts?: {
ignoreDevDependencies?: boolean;
bumpVersionsWithWorkspaceProtocolOnly?: boolean;
}
) {
const graph: Map<string, { pkg: Package; dependents: string[] }> = new Map();

const { graph: dependencyGraph } = getDependencyGraph(packages, {
bumpVersionsWithWorkspaceProtocolOnly:
opts?.bumpVersionsWithWorkspaceProtocolOnly === true,
});
const { graph: dependencyGraph } = getDependencyGraph(packages, opts);

const dependentsLookup: {
[key: string]: { pkg: Package; dependents: Array<string> };
Expand Down