Fixes publish and dist-tag scripts. #18219
Conversation
dgp1130
added
target: patch
|
@clydin, do you want to take a look at this change as well? |
Pure refactor that just pulls the Wombat registry into a separate string constant to be re-used across scripts.
We can't guarantee that the user provided these options at compile time, so all options should include `undefined` in their types. This makes the type system a little smarter and hopefully prevent future mistakes. Fortunately we were already doing the right thing, this will make sure we continue to do so in the future. I opted to use `Partial<DistTagArgs>` so that all future flags would be implicitly forced to be optional. Otherwise, we may forget to make them optional and potentially introduce a bug.
This includes a new `--registry` flag to the `dist-tag` command and defaults it to Wombat. This way NPM commands will explicitly use the correct registry.
Flag arguments provided by minimist are always strings. Both `--branchCheck` and `--versionCheck` failed to take this into account and were incorrectly typed as a result. Now boolean flags are parsed into actual boolean types which can be used more intuitively.
|
@dgp1130, please open separate patch PR for each LTS that you'd like to target. Currently active LTS are v8 and v9 |
`--tag` now must be explicitly specified rather than defaulting to `latest`. It is also validated to be one of the expected set of tags. There is a new `--tagCheck false` flag that will skip this in order to handle one-off custom tags if necessary in the future. Apparently NPM requires that all releases have tags (defaulting to `latest` if not explicitly specified), so there is no way to choose **not** to push a new tag. That means that you can't push `10.0.x` and then push `9.0.x` without either using `v9-lts` or clobbering `latest`. The `--tag` requirement should hopefully enforce this restriction to avoid a potential release footgun.
|
For the v8 branch, the nullish coalescing will need to be changed. It's still on TS 3.5. |
dgp1130
removed
the
target: lts
|
@clydin, thanks for pointing out the TS 3.5 issue on v8. Updated that PR to not use |
|
@dgp1130, seems like the CLA bot status is stalled. |
|
@alan-agius4, strange. This PR was stuck on CLA, while the other two were stuck on 'Missing required status "ci/circleci: integration"'. I did an empty amend and force pushed it, so that should re-trigger all the status checks. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This fixes a few known bugs in
publishanddist-tag, as well as a few new ones I found along the way.Ideally this would have been at least two distinct PRs, but they touched each other when it relates to registries in a way that would have been a pain with multiple PRs.