-
Notifications
You must be signed in to change notification settings - Fork 489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(config): allow to provide glob expressions for linked and ignore packages #458
Merged
emmatown
merged 7 commits into
changesets:master
from
emmenko:nm-config-glob-expressions
Oct 6, 2020
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ee97570
feat(config): allow to provide glob expressions for linked and ignore…
emmenko 2f0af89
refactor(config): use micromatch instead of minimatch
emmenko cd59270
refactor(config): address feedback
emmenko b2af53d
refactor(config): to simplify glob matching logic
emmenko 496796e
Fixed Array.isArray calls changing the type of the tested value to `a…
Andarist 8f4bcf8
Merge branch 'master' into nm-config-glob-expressions
emmatown 367589c
Create thirty-fans-rhyme.md
emmatown File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import fixturez from "fixturez"; | |
import { read, parse } from "./"; | ||
import jestInCase from "jest-in-case"; | ||
import * as logger from "@changesets/logger"; | ||
import { Config, WrittenConfig } from "@changesets/types"; | ||
import { Packages } from "@manypkg/get-packages"; | ||
|
||
jest.mock("@changesets/logger"); | ||
|
@@ -17,6 +18,14 @@ let defaultPackages: Packages = { | |
tool: "yarn" | ||
}; | ||
|
||
const withPackages = (pkgNames: string[]) => ({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Helper function for the tests |
||
...defaultPackages, | ||
packages: pkgNames.map(pkgName => ({ | ||
packageJson: { name: pkgName, version: "" }, | ||
dir: "dir" | ||
})) | ||
}); | ||
|
||
test("read reads the config", async () => { | ||
let dir = f.find("new-config"); | ||
let config = await read(dir, defaultPackages); | ||
|
@@ -49,14 +58,20 @@ let defaults = { | |
} | ||
} as const; | ||
|
||
let correctCases = { | ||
let correctCases: { | ||
emmenko marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[caseName: string]: { | ||
packages?: string[]; | ||
input: WrittenConfig; | ||
output: Config; | ||
}; | ||
} = { | ||
defaults: { | ||
input: {}, | ||
output: defaults | ||
}, | ||
"changelog string": { | ||
input: { | ||
changelog: "some-module" | ||
changelog: ["some-module", null] | ||
emmenko marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
output: { | ||
...defaults, | ||
|
@@ -126,6 +141,23 @@ let correctCases = { | |
linked: [["pkg-a", "pkg-b"]] | ||
} | ||
}, | ||
linkedWithGlobs: { | ||
packages: [ | ||
"pkg-a", | ||
"pkg-b", | ||
"@pkg/a", | ||
"@pkg/b", | ||
"@pkg-other/a", | ||
"@pkg-other/b" | ||
], | ||
input: { | ||
linked: [["pkg-*", "@pkg/*"], ["@pkg-other/a"]] | ||
}, | ||
output: { | ||
...defaults, | ||
linked: [["pkg-a", "pkg-b", "@pkg/a", "@pkg/b"], ["@pkg-other/a"]] | ||
} | ||
}, | ||
"update internal dependencies minor": { | ||
input: { | ||
updateInternalDependencies: "minor" | ||
|
@@ -152,26 +184,27 @@ let correctCases = { | |
...defaults, | ||
ignore: ["pkg-a", "pkg-b"] | ||
} | ||
}, | ||
ignoreWithGlobs: { | ||
packages: ["pkg-a", "pkg-b", "@pkg/a", "@pkg/b"], | ||
input: { | ||
ignore: ["pkg-*", "@pkg/*"] | ||
}, | ||
output: { | ||
...defaults, | ||
ignore: ["pkg-a", "pkg-b", "@pkg/a", "@pkg/b"] | ||
} | ||
} | ||
} as const; | ||
}; | ||
|
||
jestInCase( | ||
"parse", | ||
testCase => { | ||
expect( | ||
parse(testCase.input, { | ||
...defaultPackages, | ||
packages: [ | ||
{ | ||
packageJson: { name: "pkg-a", version: "" }, | ||
dir: "dir" | ||
}, | ||
{ | ||
packageJson: { name: "pkg-b", version: "" }, | ||
dir: "dir" | ||
} | ||
] | ||
}) | ||
parse( | ||
testCase.input, | ||
withPackages(testCase.packages || ["pkg-a", "pkg-b"]) | ||
) | ||
).toEqual(testCase.output); | ||
}, | ||
correctCases | ||
|
@@ -182,15 +215,18 @@ let unsafeParse = parse as any; | |
describe("parser errors", () => { | ||
test("changelog invalid value", () => { | ||
expect(() => { | ||
unsafeParse({ changelog: {} }); | ||
unsafeParse({ changelog: {} }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`changelog\` option is set as {} when the only valid values are undefined, a module path(e.g. \\"@changesets/cli/changelog\\" or \\"./some-module\\") or a tuple with a module path and config for the changelog generator(e.g. [\\"@changesets/cli/changelog\\", { someOption: true }])" | ||
`); | ||
}); | ||
test("changelog array with 3 values", () => { | ||
expect(() => { | ||
unsafeParse({ changelog: ["some-module", "something", "other"] }); | ||
unsafeParse( | ||
{ changelog: ["some-module", "something", "other"] }, | ||
defaultPackages | ||
); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`changelog\` option is set as [ | ||
|
@@ -202,7 +238,7 @@ The \`changelog\` option is set as [ | |
}); | ||
test("changelog array with first value not string", () => { | ||
expect(() => { | ||
unsafeParse({ changelog: [false, "something"] }); | ||
unsafeParse({ changelog: [false, "something"] }, defaultPackages); | ||
emmenko marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`changelog\` option is set as [ | ||
|
@@ -213,31 +249,31 @@ The \`changelog\` option is set as [ | |
}); | ||
test("access other string", () => { | ||
expect(() => { | ||
unsafeParse({ access: "something" }); | ||
unsafeParse({ access: "something" }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`access\` option is set as \\"something\\" when the only valid values are undefined, \\"public\\" or \\"restricted\\"" | ||
`); | ||
}); | ||
test("commit non-boolean", () => { | ||
expect(() => { | ||
unsafeParse({ commit: "something" }); | ||
unsafeParse({ commit: "something" }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`commit\` option is set as \\"something\\" when the only valid values are undefined or a boolean" | ||
`); | ||
}); | ||
test("linked non-array", () => { | ||
expect(() => { | ||
unsafeParse({ linked: {} }); | ||
unsafeParse({ linked: {} }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`linked\` option is set as {} when the only valid values are undefined or an array of arrays of package names" | ||
`); | ||
}); | ||
test("linked array of non array", () => { | ||
expect(() => { | ||
unsafeParse({ linked: [{}] }); | ||
unsafeParse({ linked: [{}] }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`linked\` option is set as [ | ||
|
@@ -247,7 +283,7 @@ The \`linked\` option is set as [ | |
}); | ||
test("linked array of array of non-string", () => { | ||
expect(() => { | ||
unsafeParse({ linked: [[{}]] }); | ||
unsafeParse({ linked: [[{}]] }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`linked\` option is set as [ | ||
|
@@ -257,64 +293,67 @@ The \`linked\` option is set as [ | |
] when the only valid values are undefined or an array of arrays of package names" | ||
`); | ||
}); | ||
test("linked pacakge that does not exist", () => { | ||
test("linked package that does not exist", () => { | ||
expect(() => { | ||
parse({ linked: [["not-existing"]] }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The package or glob expression \\"not-existing\\" specified in the \`linked\` option does not match any package in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/minimatch." | ||
`); | ||
}); | ||
test("linked package that does not exist (using glob expressions)", () => { | ||
expect(() => { | ||
parse({ linked: [["pkg-a"]] }, defaultPackages); | ||
parse({ linked: [["pkg-a", "foo/*"]] }, withPackages(["pkg-a"])); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The package \\"pkg-a\\" is specified in the \`linked\` option but it is not found in the project. You may have misspelled the package name." | ||
The package or glob expression \\"foo/*\\" specified in the \`linked\` option does not match any package in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/minimatch." | ||
`); | ||
}); | ||
test("linked package in two linked groups", () => { | ||
expect(() => { | ||
parse({ linked: [["pkg-a"], ["pkg-a"]] }, withPackages(["pkg-a"])); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The package \\"pkg-a\\" is defined in multiple sets of linked packages. Packages can only be defined in a single set of linked packages. If you are using glob expressions, make sure that they are valid according to https://www.npmjs.com/package/minimatch." | ||
`); | ||
}); | ||
test("linked package in two linked groups (using glob expressions)", () => { | ||
expect(() => { | ||
parse( | ||
{ linked: [["pkg-a"], ["pkg-a"]] }, | ||
{ | ||
...defaultPackages, | ||
packages: [ | ||
{ | ||
packageJson: { name: "pkg-a", version: "" }, | ||
dir: "dir" | ||
} | ||
] | ||
} | ||
{ linked: [["pkg-*"], ["pkg-*"]] }, | ||
withPackages(["pkg-a", "pkg-b"]) | ||
); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The package \\"pkg-a\\" is in multiple sets of linked packages. Packages can only be in a single set of linked packages." | ||
The package \\"pkg-a\\" is defined in multiple sets of linked packages. Packages can only be defined in a single set of linked packages. If you are using glob expressions, make sure that they are valid according to https://www.npmjs.com/package/minimatch. | ||
The package \\"pkg-b\\" is defined in multiple sets of linked packages. Packages can only be defined in a single set of linked packages. If you are using glob expressions, make sure that they are valid according to https://www.npmjs.com/package/minimatch." | ||
`); | ||
}); | ||
test("access private warns and sets to restricted", () => { | ||
let config = unsafeParse({ access: "private" }, []); | ||
let config = unsafeParse({ access: "private" }, defaultPackages); | ||
expect(config).toEqual(defaults); | ||
expect(logger.warn).toBeCalledWith( | ||
'The `access` option is set as "private", but this is actually not a valid value - the correct form is "restricted".' | ||
); | ||
}); | ||
test("updateInternalDependencies not patch or minor", () => { | ||
expect(() => { | ||
unsafeParse({ updateInternalDependencies: "major" }); | ||
unsafeParse({ updateInternalDependencies: "major" }, defaultPackages); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`updateInternalDependencies\` option is set as \\"major\\" but can only be 'patch' or 'minor'" | ||
`); | ||
}); | ||
test("ignore non-array", () => { | ||
expect(() => | ||
unsafeParse({ | ||
ignore: "string value" | ||
}) | ||
).toThrowErrorMatchingInlineSnapshot(` | ||
expect(() => unsafeParse({ ignore: "string value" }, defaultPackages)) | ||
.toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`ignore\` option is set as \\"string value\\" when the only valid values are undefined or an array of package names" | ||
`); | ||
}); | ||
test("ignore array of non-string", () => { | ||
expect(() => | ||
unsafeParse({ | ||
ignore: [123, "pkg-a"] | ||
}) | ||
).toThrowErrorMatchingInlineSnapshot(` | ||
expect(() => unsafeParse({ ignore: [123, "pkg-a"] }, defaultPackages)) | ||
.toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`ignore\` option is set as [ | ||
123, | ||
|
@@ -323,24 +362,23 @@ The \`ignore\` option is set as [ | |
`); | ||
}); | ||
test("ignore package that does not exist", () => { | ||
expect(() => | ||
parse( | ||
{ | ||
ignore: ["pkg-a"] | ||
}, | ||
defaultPackages | ||
) | ||
).toThrowErrorMatchingInlineSnapshot(` | ||
expect(() => unsafeParse({ ignore: ["pkg-a"] }, defaultPackages)) | ||
.toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The package or glob expression \\"pkg-a\\" is specified in the \`ignore\` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/minimatch." | ||
`); | ||
}); | ||
test("ignore package that does not exist (using glob expressions)", () => { | ||
expect(() => unsafeParse({ ignore: ["pkg-*"] }, defaultPackages)) | ||
.toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The package \\"pkg-a\\" is specified in the \`ignore\` option but it is not found in the project. You may have misspelled the package name." | ||
The package or glob expression \\"pkg-*\\" is specified in the \`ignore\` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/minimatch." | ||
`); | ||
}); | ||
test("ingore missing dependent packages", async () => { | ||
expect(() => | ||
parse( | ||
{ | ||
ignore: ["pkg-b"] | ||
}, | ||
unsafeParse( | ||
{ ignore: ["pkg-b"] }, | ||
{ | ||
...defaultPackages, | ||
packages: [ | ||
|
@@ -367,23 +405,29 @@ The package \\"pkg-a\\" depends on the ignored package \\"pkg-b\\", but \\"pkg-a | |
|
||
test("onlyUpdatePeerDependentsWhenOutOfRange non-boolean", () => { | ||
expect(() => { | ||
unsafeParse({ | ||
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { | ||
onlyUpdatePeerDependentsWhenOutOfRange: "not true" | ||
} | ||
}); | ||
unsafeParse( | ||
{ | ||
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { | ||
onlyUpdatePeerDependentsWhenOutOfRange: "not true" | ||
} | ||
}, | ||
defaultPackages | ||
); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`onlyUpdatePeerDependentsWhenOutOfRange\` option is set as \\"not true\\" when the only valid values are undefined or a boolean" | ||
`); | ||
}); | ||
test("useCalculatedVersionForSnapshots non-boolean", () => { | ||
expect(() => { | ||
unsafeParse({ | ||
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { | ||
useCalculatedVersionForSnapshots: "not true" | ||
} | ||
}); | ||
unsafeParse( | ||
{ | ||
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: { | ||
useCalculatedVersionForSnapshots: "not true" | ||
} | ||
}, | ||
defaultPackages | ||
); | ||
}).toThrowErrorMatchingInlineSnapshot(` | ||
"Some errors occurred when validating the changesets config: | ||
The \`useCalculatedVersionForSnapshots\` option is set as \\"not true\\" when the only valid values are undefined or a boolean" | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use
micromatch
instead? (micromatch is what fast-glob uses which globby uses which is what @manypkg/get-packages uses which is what changesets uses for getting the packages in a monorepo so it's good to use the same thing for consistency and not spending the time loading two glob implementations when people use Changesets)(this comment is assuming we go with globs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, I thought I read "minimatch" from your comment, but it was "micromatch" instead. 🤦♂️
My bad sorry, I'll change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 2f0af89