Skip to content
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

(build-tools): Add command to update types #17427

Merged

Conversation

sonalideshpandemsft
Copy link
Contributor

@sonalideshpandemsft sonalideshpandemsft commented Sep 21, 2023

This new flub command will be used to update the package.json types to point to the alpha/beta release as required.

This command first verifies the presence of an api-extractor.json file and subsequently examines its rollup configurations. If these configurations are present, it proceeds to update the package.json's types/typings field based on predefined conditions and flags.

The updated package.json is serialized while retaining its original formatting and then written back to the file.

flub release setPackageTypesField -g client --types alpha --json
flub release setPackageTypesField -g client --types beta --json
flub release setPackageTypesField -g client --types public --json
flub release setPackageTypesField -g client --types untrimmed --json

AB#5535

@github-actions github-actions bot added area: build Build related issues base: main PRs targeted against main branch labels Sep 21, 2023
Copy link
Member

@tylerbutler tylerbutler left a comment

Choose a reason for hiding this comment

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

Think about how to test this. If you factor out the main code into functions that accept a Package, then you can write tests that check that the properties are set correctly by the functions, for example.

@sonalideshpandemsft sonalideshpandemsft force-pushed the cmd-to-update-types branch 2 times, most recently from 21b9219 to 6a1e16b Compare September 22, 2023 06:26
@CraigMacomber
Copy link
Contributor

How have you been testing this? Overall direction looks good, but I think the data you pass around between functions can maybe be simplified and I'm not clear exactly how this will get used in the CI. Maybe this PR is not the right one, but ultimately I want to know how and when we're going to publish alpha/beta builds. Is the whole release group considered an alpha/beta release, and that's decided when we run the release pipeline? Or are we planning to run a second pipeline that publishes alpha/beta packages? Something else?

  1. How and when we're going to publish alpha/beta builds?
    Afaik, alpha/beta builds would be usually published on demand.
    cc: @CraigMacomber, looking forward to your thoughts on this questions
  2. Is the whole release group considered an alpha/beta release, and that's decided when we run the release pipeline? Or are we planning to run a second pipeline that publishes alpha/beta packages?
    The entire release group is labeled as an alpha/beta release based on the chosen Release Build type (alpha/beta) when running the release pipeline.

This makes me wonder, is it possible to release a single package say @fluid-experimental/tree2 and not the entire client group?

  1. For now, manually when a developer decides to make one and give it to a customer for testing. Eventually we might formalize this a bit more, but the main purpose is to be able to iterate on an API with a customer or a few customers productively (so in main, without having to semver stabilize it until after iterating a few times), and manual releases are fine for that (we need a personal relation with the customer using it to apply their feedback anyway).
  2. I think doing this at the release group level makes sense. It ensures the packages being released have corresponding packages to depend on, and allows alpha apis in one package to depend on alpha apis in another, which is I think is how API-extractor assumes things works and seems like what we want (we need it for testing out APIs that span multiple packages for example)

packagesNotUpdated: string[];
}

const knownDtsKinds = ["alpha", "beta", "public", "untrimmed"] as const;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you should refer to these as dtsTypes. Multiple .d.ts files being involved here is really an implementation detail of how this system you are making works with API-extractor.

I think it would make more sense to name thins (like this command, and this list) after "releaseTags" and link https://api-extractor.com/pages/tsdoc/doc_comment_syntax/#release-tags for additional docs.

Naming the command something like "selectReleaseTagToPublish" would be much more clear.

Copy link
Member

Choose a reason for hiding this comment

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

"release" is a very overloaded term. I don't want to use it for this purpose. This work is explicitly about types only. Nothing related to what we release when, what version the release will have, etc. This tool only changes the types between "flavors" of the types.

Copy link
Member

Choose a reason for hiding this comment

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

It also has nothing to do with "publishing," so I don't think setReleaseTagToPublish is a good choice either. Maybe setPackageTypesField?

type DtsKind = typeof knownDtsKinds[number];

function isDtsKind(str: string | undefined): str is DtsKind {
return str === undefined ? false : knownDtsKinds.includes(str as any);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this as any cast needed? Might need to explicitly type knownDtsKinds as string[]?

@sonalideshpandemsft sonalideshpandemsft force-pushed the cmd-to-update-types branch 2 times, most recently from 9823f13 to db6323e Compare October 5, 2023 01:29
@sonalideshpandemsft sonalideshpandemsft merged commit 0bef3a0 into microsoft:main Oct 10, 2023
27 checks passed
@sonalideshpandemsft sonalideshpandemsft deleted the cmd-to-update-types branch October 10, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants