feat: use c12 to replace cosmiconfig #1212
Conversation
# Conflicts: # package-lock.json # test/config.js # test/git.init.js # test/git.js # test/github.js # test/gitlab.js # test/npm.js # test/prompt.js # test/shell.js # test/spinner.js # test/tasks.interactive.js # test/tasks.js # test/util/index.js # test/version.js
|
Hi @webpro, I think the biggest difference is that c12 supports nested extends and multiple extends, for example: In my-project, extends two configs // my-project/.release-it.json
{
"extends": [
"github:owner/repo",
"github:foo/bar",
]
}These two configs also extended from other repos # github/owner/repo/.release-it.toml
extends = [
"github:other/repo"
]
[git]
commitMessage = "Release version ${version}"# github/other/repo/.release-it.yaml
gitlab:
release: trueAnother key point in the example above is that extend doesn't have to be in a JSON file. So switching to c12 will make the functionality more complete and increase flexibility. |
|
Thanks for the explanation, that makes sense. The async config loading adds a lot of code overall, but I guess sooner or later this had to happen anyway. @juancarlosjr97 What do you think? |
commit: |
package.json
Outdated
| @@ -94,11 +94,13 @@ | |||
| "issue-parser": "7.0.1", | |||
| "lodash.get": "4.4.2", | |||
| "lodash.merge": "4.6.2", | |||
| "make-synchronized": "0.6.0", | |||
There was a problem hiding this comment.
Sorry my bad, that is try to make load config to synchronized, but that have some problems
I'm already drop that at d3e7fbb
|
@webpro There tests have some problems about mock fetch response, here is the PR humanwhocodes/mentoss#92 |
| - `github:owner/repo#tag`: Get the config from the specified tag in the repo at Github. | ||
| - `github:owner/repo:subdir#tag`: Get the config from the specified tag in the repo sub dir at Github. | ||
|
|
||
| And support other schema, either `gitlab:`, `bitbucket:`, or `https:`. |
There was a problem hiding this comment.
If it is HTTPS, then the file should be public and not behind authorization. However, GitHub, GitLab, and BitBucket will work as it is getting pulled with the token, right?
There was a problem hiding this comment.
Yes you right, if want to get private repo or private content from https, it could use the environment GIGET_AUTH to set it. The value will be set in Authorization: Bearer ... header
Refs to giget docs
There was a problem hiding this comment.
Cool! Then I am super happy! I did not know we were working with giget, but I can see it is a transitive dependency of c12, so awesome :)
|
Everything looks good, a very nice improvement, only one question. |
|
Also, this will make this change https://github.com/release-it/release-it/pull/1211/files not longer needed @webpro I believe |
True! Thanks for chiming in, very glad to have your opinion on this one. |
|
About the const config = new Config();
await config.resolved;Is this a well-known pattern to add async logic to the constructor? If not, how about this: const config = new Config();
await config.resolve(); // or await config.init();So that the async part isn't automatically triggered from the For me personally this latter pattern feels more natural, but happy to hear your thoughts. |
|
@webpro I think that make more sense as you say. I like this pattern to use |
# Conflicts: # package-lock.json # package.json
# Conflicts: # package-lock.json # package.json
|
Thank you @aa900031! Also for the fix in Mentoss 🔥 |
|
🚀 This pull request is included in v19.0.0-next.5. See Release 19.0.0-next.5 for release notes. |
Follow that issue #934, we can use c12 to replace cosmiconfig.
Changes
options, andlocalConfigis not sync anymore inConfig:Because the loadConfig provided by c12 is processed asynchronously, we need to change the way obtains
optionsandlocalConfigin classConfig.For the above reasons, an additional
resolvedvariable is provided so that we can await it and ensure thatoptionsandlocalConfighave been obtained correctly.