diff --git a/.changeset/cyan-cups-push.md b/.changeset/cyan-cups-push.md new file mode 100644 index 0000000..a6ac92f --- /dev/null +++ b/.changeset/cyan-cups-push.md @@ -0,0 +1,31 @@ +--- +"@manypkg/cli": minor +--- + +Add new check: INTERNAL_DEV_DEP_NOT_STAR + +This check moves internal devDependencies between packages to be `*` - so in a case where I had a package sunshine, which depends on internal package 'sun': + +```json +{ + "name": "sunshine", + "version": "1.0.0", + "devDependencies": { + "sun": "^1.0.0" + } +} +``` + +we will now have: + +```json +{ + "name": "sunshine", + "version": "1.0.0", + "devDependencies": { + "sun": "*" + } +} +``` + +This is because all internal dependencies are always linked if the version of the internal dependency is within the specified range(which is already enforced by Manypkg), and devDependencies are only relevant in local installs. Having set versions here caused packages to be patched when one of their devDependencies left the range, which was not strictly necessary. diff --git a/README.md b/README.md index 5fedc46..1862cd1 100644 --- a/README.md +++ b/README.md @@ -40,12 +40,20 @@ The highest range of the dependency is set as the range at every non-peer depend ## Internal mismatch -The ranges for all dependencies(excluding `peerDependencies`) on internal packages should include the version of the internal package. This is so that an internal package will never depend on another internal package but get the package from the registry because that happening is very confusing and you should always prefer a local version of any given package. +The ranges for all regular dependencies and optionalDependencies(not peerDependencies or devDependencies) on internal packages should include the version of the internal package. This is so that an internal package will never depend on another internal package but get the package from the registry because that happening is very confusing and you should always prefer a local version of any given package. ### How it's fixed If the range is a [caret range](https://github.com/npm/node-semver#caret-ranges-123-025-004) or a [tilde range](https://github.com/npm/node-semver#tilde-ranges-123-12-1) with no other comparators, the range is set as a caret or tilde range respectively with the version of the internal package. If it is any other range, the range is set to the exact version of the internal package. +## Internal devDependencies are not `*` + +The ranges for devDependencies should not matter, and having non-star versions causes accidental patching of packages when no changes have been made. + +### How it's fixed + +Change whatever other version is specified to be `*` + ## Invalid dev and peer dependency relationship All `peerDependencies` should also be specified in `devDependencies` and the range specified in `devDependencies` should be a subset of the range for that dependency in `peerDependencies`. This is so that `peerDependencies` are available in the package during development for testing and etc. @@ -85,4 +93,3 @@ When you add a package with `yarn add` or etc. dependencies are sorted, and this #### How it's fixed This is fixed by sorting deps by key alphabetically. - diff --git a/packages/cli/README.md b/packages/cli/README.md index 6d41b66..0bb8743 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -20,4 +20,4 @@ yarn manypkg check yarn manypkg fix ``` -See more details at https://github.com/mitchellhamilton/manypkg +See more details at https://github.com/thinkmill/manypkg diff --git a/packages/cli/package.json b/packages/cli/package.json index dc61932..cd9898c 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -15,10 +15,10 @@ "find-up": "^4.1.0", "find-workspaces-root": "^0.1.0", "fs-extra": "^8.1.0", - "get-workspaces": "^0.4.0", + "get-workspaces": "^0.5.0", "meow": "^5.0.0", "p-limit": "^2.2.1", - "sembear": "^0.4.0", + "sembear": "^0.4.1", "semver": "^6.3.0", "spawndamnit": "^2.0.0", "validate-npm-package-name": "^3.0.0" @@ -32,4 +32,4 @@ "script-runner" ] } -} \ No newline at end of file +} diff --git a/packages/cli/src/checks/INTERNAL_DEV_DEP_NOT_STAR.ts b/packages/cli/src/checks/INTERNAL_DEV_DEP_NOT_STAR.ts new file mode 100644 index 0000000..26376fc --- /dev/null +++ b/packages/cli/src/checks/INTERNAL_DEV_DEP_NOT_STAR.ts @@ -0,0 +1,43 @@ +import { makeCheck, Workspace } from "./utils"; + +export type ErrorType = { + type: "INTERNAL_DEV_DEP_NOT_STAR"; + workspace: Workspace; + dependencyWorkspace: Workspace; +}; + +export default makeCheck({ + validate: (workspace, allWorkspaces) => { + let errors: ErrorType[] = []; + let deps = workspace.config.devDependencies; + if (deps) { + for (let depName in deps) { + let range = deps[depName]; + let dependencyWorkspace = allWorkspaces.get(depName); + if (dependencyWorkspace !== undefined && range !== "*") { + errors.push({ + type: "INTERNAL_DEV_DEP_NOT_STAR", + workspace, + dependencyWorkspace + }); + } + } + } + + return errors; + }, + fix: error => { + let deps = error.workspace.config.devDependencies; + if (deps && deps[error.dependencyWorkspace.name]) { + deps[error.dependencyWorkspace.name] = "*"; + } + return { requiresInstall: true }; + }, + print: error => + `${error.workspace.name} has a dependency on ${ + error.dependencyWorkspace.name + } as a devDependency, but has the version listed as ${ + error.workspace.config.devDependencies![error.dependencyWorkspace.name] + }. Please update the dependency to be "*"`, + type: "all" +}); diff --git a/packages/cli/src/checks/INTERNAL_MISMATCH.ts b/packages/cli/src/checks/INTERNAL_MISMATCH.ts index 6eec785..1bd6a16 100644 --- a/packages/cli/src/checks/INTERNAL_MISMATCH.ts +++ b/packages/cli/src/checks/INTERNAL_MISMATCH.ts @@ -6,7 +6,7 @@ import { } from "./utils"; import semver from "semver"; -type ErrorType = { +export type ErrorType = { type: "INTERNAL_MISMATCH"; workspace: Workspace; dependencyWorkspace: Workspace; @@ -17,6 +17,8 @@ export default makeCheck({ validate: (workspace, allWorkspaces) => { let errors: ErrorType[] = []; for (let depType of NORMAL_DEPENDENCY_TYPES) { + // devDependencies are handled by INTERNAL_DEV_DEP_NOT_STAR + if (depType === "devDependencies") continue; let deps = workspace.config[depType]; if (deps) { for (let depName in deps) { @@ -36,6 +38,7 @@ export default makeCheck({ } } } + return errors; }, fix: error => { diff --git a/packages/cli/src/checks/__tests__/INTERNAL_DEV_DEP_NOT_STAR.ts b/packages/cli/src/checks/__tests__/INTERNAL_DEV_DEP_NOT_STAR.ts new file mode 100644 index 0000000..a5187d2 --- /dev/null +++ b/packages/cli/src/checks/__tests__/INTERNAL_DEV_DEP_NOT_STAR.ts @@ -0,0 +1,66 @@ +import makeCheck, { ErrorType } from "../INTERNAL_DEV_DEP_NOT_STAR"; +import { Workspace } from "get-workspaces"; + +let getFakeWS = ( + name: string = "pkg-1", + version: string = "1.0.0" +): Workspace => { + return { + name, + dir: `some/fake/dir/${name}`, + config: { + name, + version + } + }; +}; + +let getWS = (): Map => { + let ws = new Map(); + ws.set("pkg-1", getFakeWS()); + return ws; +}; + +describe("internal dev ep is not star", () => { + it("should not error if internal version is *", () => { + let ws = getWS(); + let dependsOnOne = getFakeWS("depends-on-one"); + dependsOnOne.config.devDependencies = { + "pkg-1": "*" + }; + ws.set("depends-on-one", dependsOnOne); + let errors = makeCheck.validate(dependsOnOne, ws); + expect(errors.length).toEqual(0); + }); + it("should error if internal version is not *", () => { + let ws = getWS(); + let dependsOnOne = getFakeWS("depends-on-one"); + dependsOnOne.config.devDependencies = { + "pkg-1": "^1.0.0" + }; + ws.set("depends-on-one", dependsOnOne); + let errors = makeCheck.validate(dependsOnOne, ws); + expect(errors[0]).toMatchObject({ + type: "INTERNAL_DEV_DEP_NOT_STAR", + workspace: dependsOnOne, + dependencyWorkspace: ws.get("pkg-1") + }); + expect(errors.length).toEqual(1); + }); + it("should fix an incompatible version", () => { + let workspace = getFakeWS("depends-on-one"); + workspace.config.devDependencies = { + "pkg-1": "^1.0.0" + }; + let error: ErrorType = { + type: "INTERNAL_DEV_DEP_NOT_STAR", + workspace, + dependencyWorkspace: getFakeWS() + }; + let result = makeCheck.fix!(error); + expect(result).toMatchObject({ requiresInstall: true }); + expect(error.workspace.config.devDependencies).toMatchObject({ + "pkg-1": "*" + }); + }); +}); diff --git a/packages/cli/src/checks/__tests__/INTERNAL_MISMATCH.ts b/packages/cli/src/checks/__tests__/INTERNAL_MISMATCH.ts new file mode 100644 index 0000000..62891fd --- /dev/null +++ b/packages/cli/src/checks/__tests__/INTERNAL_MISMATCH.ts @@ -0,0 +1,85 @@ +import makeCheck, { ErrorType } from "../INTERNAL_MISMATCH"; +import { Workspace } from "get-workspaces"; + +let getFakeWS = ( + name: string = "pkg-1", + version: string = "1.0.0" +): Workspace => { + return { + name, + dir: `some/fake/dir/${name}`, + config: { + name, + version + } + }; +}; + +let getWS = (): Map => { + let ws = new Map(); + ws.set("pkg-1", getFakeWS()); + return ws; +}; + +describe("internal mismatch", () => { + it("should not error if internal version is compatible", () => { + let ws = getWS(); + let dependsOnOne = getFakeWS("depends-on-one"); + dependsOnOne.config.dependencies = { + "pkg-1": "^1.0.0" + }; + ws.set("depends-on-one", dependsOnOne); + let errors = makeCheck.validate(dependsOnOne, ws); + expect(errors.length).toEqual(0); + }); + it("should error if internal version is not compatible", () => { + let ws = getWS(); + let dependsOnOne = getFakeWS("depends-on-one"); + dependsOnOne.config.dependencies = { + "pkg-1": "^0.1.0" + }; + ws.set("depends-on-one", dependsOnOne); + let errors = makeCheck.validate(dependsOnOne, ws); + expect(errors[0]).toMatchObject({ + type: "INTERNAL_MISMATCH", + dependencyWorkspace: ws.get("pkg-1"), + workspace: dependsOnOne, + dependencyRange: "^0.1.0" + }); + expect(errors.length).toEqual(1); + }); + it("should fix an incompatible version", () => { + let workspace = getFakeWS("depends-on-one"); + workspace.config.dependencies = { + "pkg-1": "^0.1.0" + }; + + let error: ErrorType = { + type: "INTERNAL_MISMATCH", + workspace, + dependencyWorkspace: getFakeWS(), + dependencyRange: "^0.1.0" + }; + + let fixed = makeCheck.fix!(error); + expect(fixed).toMatchObject({ requiresInstall: true }); + expect(workspace.config.dependencies).toMatchObject({ "pkg-1": "^1.0.0" }); + }); + it("should not check dev dependencies", () => { + // This is handled by INTERNAL_DEV_DEP_NOT_STAR + let ws = getWS(); + let dependsOnOne = getFakeWS("depends-on-one"); + dependsOnOne.config.devDependencies = { + "pkg-1": "^0.1.0" + }; + ws.set("depends-on-one", dependsOnOne); + let errors = makeCheck.validate(dependsOnOne, ws); + expect(errors.length).toEqual(0); + }); + // NB this is the same as dependency right now, but I added it for completeness + describe.skip("peerDependency", () => { + it("should not error if internal version is compatible", () => {}); + it("should error if internal version is not compatible", () => {}); + it("should fix an incompatible version", () => {}); + }); +}); diff --git a/packages/cli/src/checks/index.ts b/packages/cli/src/checks/index.ts index 07a7e0d..53e04bb 100644 --- a/packages/cli/src/checks/index.ts +++ b/packages/cli/src/checks/index.ts @@ -1,5 +1,6 @@ import EXTERNAL_MISMATCH from "./EXTERNAL_MISMATCH"; import INTERNAL_MISMATCH from "./INTERNAL_MISMATCH"; +import INTERNAL_DEV_DEP_NOT_STAR from "./INTERNAL_DEV_DEP_NOT_STAR"; import INVALID_DEV_AND_PEER_DEPENDENCY_RELATIONSHIP from "./INVALID_DEV_AND_PEER_DEPENDENCY_RELATIONSHIP"; import INVALID_PACKAGE_NAME from "./INVALID_PACKAGE_NAME"; import MULTIPLE_DEPENDENCY_TYPES from "./MULTIPLE_DEPENDENCY_TYPES"; @@ -9,6 +10,7 @@ import UNSORTED_DEPENDENCIES from "./UNSORTED_DEPENDENCIES"; export let checks = [ EXTERNAL_MISMATCH, INTERNAL_MISMATCH, + INTERNAL_DEV_DEP_NOT_STAR, INVALID_DEV_AND_PEER_DEPENDENCY_RELATIONSHIP, INVALID_PACKAGE_NAME, MULTIPLE_DEPENDENCY_TYPES, diff --git a/test-gatsby/package.json b/test-gatsby/package.json index 85076c2..7cec0b8 100644 --- a/test-gatsby/package.json +++ b/test-gatsby/package.json @@ -2,9 +2,6 @@ "name": "test-gatsby-thing", "private": true, "version": "0.0.1", - "peerDependencies": { - "gatsby": "^2.15.28" - }, "dependencies": { "@manypkg/gatsby-source-workspace": "0.1.0", "@mdx-js/mdx": "^1.5.1", diff --git a/yarn.lock b/yarn.lock index 83251e8..6c9244b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6130,14 +6130,6 @@ get-value@^2.0.3, get-value@^2.0.6: resolved "https://registry.yarnpkg.com/get-value/-/get-value-2.0.6.tgz#dc15ca1c672387ca76bd37ac0a395ba2042a2c28" integrity sha1-3BXKHGcjh8p2vTesCjlbogQqLCg= -get-workspaces@^0.4.0: - version "0.4.0" - resolved "https://registry.yarnpkg.com/get-workspaces/-/get-workspaces-0.4.0.tgz#512dedc0e9a96b0c17035af363236299247f4cc8" - integrity sha512-O4pSctjkmrLRs8GxnaQ/xy16PWoZpEUzpgtBAj9UIYNrtag1rRnFOyvJULIMPhpbZKOlswmrS5Ku6pjt35Yj+g== - dependencies: - fs-extra "^7.0.1" - globby "^9.2.0" - get-workspaces@^0.5.0: version "0.5.0" resolved "https://registry.yarnpkg.com/get-workspaces/-/get-workspaces-0.5.0.tgz#bc00b6d158f409a70ba54cc95e58bf29cfe4df1e" @@ -6220,6 +6212,12 @@ global-prefix@^1.0.1: version "1.0.2" resolved "https://registry.yarnpkg.com/global-prefix/-/global-prefix-1.0.2.tgz#dbf743c6c14992593c655568cb66ed32c0122ebe" integrity sha1-2/dDxsFJklk8ZVVoy2btMsASLr4= + dependencies: + expand-tilde "^2.0.2" + homedir-polyfill "^1.0.1" + ini "^1.3.4" + is-windows "^1.0.1" + which "^1.2.14" global@^4.3.0: version "4.4.0" @@ -11881,10 +11879,10 @@ selfsigned@^1.10.7: dependencies: node-forge "0.9.0" -sembear@^0.4.0: - version "0.4.0" - resolved "https://registry.yarnpkg.com/sembear/-/sembear-0.4.0.tgz#19a140d659205069f5f9003edb9671d545ffa5c1" - integrity sha512-CHBII+H79ZAKU/85p4wS0JFTYuKCzzHffQ3NsLnMcpRjjx3ZcNE8CLuNa2QnxWDwXskg8ey8jJbfCOf5E2Lx4A== +sembear@^0.4.1: + version "0.4.1" + resolved "https://registry.yarnpkg.com/sembear/-/sembear-0.4.1.tgz#c1599bba0fd29cce68a25382576891e5fae84990" + integrity sha512-q3LJepi7qgDbkR9eDe1OVYfbD8A14vuC8TtETtEvpAQ4GxynA9B0fhG6ts8scBMDe/GwLlVUeU+Icnl1oidH7g== dependencies: "@types/semver" "^6.0.1" semver "^6.3.0"