Skip to content

Commit

Permalink
Merge branch 'main' into ai-tong
Browse files Browse the repository at this point in the history
  • Loading branch information
chentong7 authored Oct 29, 2024
2 parents 9354a87 + 2d98e6c commit bffa806
Show file tree
Hide file tree
Showing 71 changed files with 2,570 additions and 899 deletions.
34 changes: 34 additions & 0 deletions .changeset/clever-birds-wear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
"@fluidframework/merge-tree": minor
"@fluidframework/tree": minor
"fluid-framework": minor
---
---
"section": fix
---

Fix compiler errors when building with libCheck

Compiling code using Fluid Framework when using TypeScript's `libCheck` (meaning without [skipLibCheck](https://www.typescriptlang.org/tsconfig/#skipLibCheck)), two compile errors can be encountered:

```
> tsc
node_modules/@fluidframework/merge-tree/lib/client.d.ts:124:18 - error TS2368: Type parameter name cannot be 'undefined'.
124 walkSegments<undefined>(handler: ISegmentAction<undefined>, start?: number, end?: number, accum?: undefined, splitRange?: boolean): void;
~~~~~~~~~
node_modules/@fluidframework/tree/lib/util/utils.d.ts:5:29 - error TS7016: Could not find a declaration file for module '@ungap/structured-clone'. 'node_modules/@ungap/structured-clone/esm/index.js' implicitly has an 'any' type.
Try `npm i --save-dev @types/ungap__structured-clone` if it exists or add a new declaration (.d.ts) file containing `declare module '@ungap/structured-clone';`
5 import structuredClone from "@ungap/structured-clone";
~~~~~~~~~~~~~~~~~~~~~~~~~
```

The first error impacts projects using TypeScript 5.5 or greater and either of the `fluid-framework` or `@fluidframework/merge-tree` packages.
The second error impacts projects using the `noImplicitAny` tsconfig setting and the `fluid-framework` or `@fluidframework/tree` packages.

Both have been fixed.

This should allow `libCheck` to be reenabled in any impacted projects.
29 changes: 29 additions & 0 deletions .changeset/few-cobras-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
"fluid-framework": minor
"@fluidframework/tree": minor
---
---
"section": tree
---

Add `.schema` member to alpha enum schema APIs

The return value from `@alpha` APIs `enumFromStrings` and `adaptEnum` now has a property named `schema` which can be used to include it in a parent schema.
This replaces the use of `typedObjectValues` which has been removed.

Use of these APIs now look like:

```typescript
const schemaFactory = new SchemaFactory("com.myApp");
const Mode = enumFromStrings(schemaFactory, ["Fun", "Cool"]);
type Mode = NodeFromSchema<(typeof Mode.schema)[number]>;
class Parent extends schemaFactory.object("Parent", { mode: Mode.schema }) {}
```


Previously the last two lines would have been:

```typescript
type Mode = NodeFromSchema<(typeof Mode)[keyof typeof Mode]>; // This no longer works
class Parent extends schemaFactory.object("Parent", { mode: typedObjectValues(Mode) }) {} // This no longer works
```
66 changes: 66 additions & 0 deletions .changeset/shy-dots-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
---
"fluid-framework": minor
"@fluidframework/tree": minor
---
---
"section": tree
---

Improve strictness of input tree types when non-exact schema are provided

Consider the following code where the type of the schema is not exactly specified:

```typescript
const schemaFactory = new SchemaFactory("com.myApp");
class A extends schemaFactory.object("A", {}) {}
class B extends schemaFactory.array("B", schemaFactory.number) {}

// Gives imprecise type (typeof A | typeof B)[]. The desired precise type here is [typeof A, typeof B].
const schema = [A, B];

const config = new TreeViewConfiguration({ schema });
const view = sharedTree.viewWith(config);

// Does not compile since setter for root is typed `never` due to imprecise schema.
view.root = [];
```

The assignment of `view.root` is disallowed since a schema with type `(typeof A | typeof B)[]` could be any of:

```typescript
const schema: (typeof A | typeof B)[] = [A];
```

```typescript
const schema: (typeof A | typeof B)[] = [B];
```

```typescript
const schema: (typeof A | typeof B)[] = [A, B];
```

The attempted assignment is not compatible with all of these (specifically it is incompatible with the first one) so performing this assignment could make the tree out of schema and is thus disallowed.

To avoid this ambiguity and capture the precise type of `[typeof A, typeof B]`, use one of the following patterns:

```typescript
const schema = [A, B] as const;
const config = new TreeViewConfiguration({ schema });
```

```typescript
const config = new TreeViewConfiguration({ schema: [A, B] });
```

To help update existing code which accidentally depended on this bug, an `@alpha` API `unsafeArrayToTuple` has been added.
Many usages of this API will produce incorrectly typed outputs.
However, when given `AllowedTypes` arrays which should not contain any unions, but that were accidentally flattened to a single union, it can fix them:

```typescript
// Gives imprecise type (typeof A | typeof B)[]
const schemaBad = [A, B];
// Fixes the type to be [typeof A, typeof B]
const schema = unsafeArrayToTuple(schemaBad);

const config = new TreeViewConfiguration({ schema });
```
2 changes: 0 additions & 2 deletions build-tools/packages/build-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@
"mdast-util-heading-range": "^4.0.0",
"mdast-util-to-string": "^4.0.0",
"minimatch": "^7.4.6",
"node-fetch": "^3.3.2",
"npm-check-updates": "^16.14.20",
"oclif": "^4.15.16",
"prettier": "~3.2.5",
Expand Down Expand Up @@ -156,7 +155,6 @@
"@types/mdast": "^4.0.4",
"@types/mocha": "^10.0.9",
"@types/node": "^18.19.59",
"@types/node-fetch": "^2.6.11",
"@types/prettier": "^2.7.3",
"@types/prompts": "^2.4.9",
"@types/semver": "^7.5.8",
Expand Down
59 changes: 55 additions & 4 deletions build-tools/packages/build-cli/src/commands/release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
*/

import { strict as assert } from "node:assert";
import { VersionBumpType, detectVersionScheme } from "@fluid-tools/version-tools";
import {
VersionBumpType,
bumpVersionScheme,
detectVersionScheme,
} from "@fluid-tools/version-tools";
import { rawlist } from "@inquirer/prompts";
import { Config } from "@oclif/core";
import chalk from "chalk";

Expand All @@ -23,7 +28,7 @@ import {
} from "../handlers/index.js";
import { PromptWriter } from "../instructionalPromptWriter.js";
// eslint-disable-next-line import/no-deprecated
import { MonoRepoKind } from "../library/index.js";
import { MonoRepoKind, getDefaultBumpTypeForBranch } from "../library/index.js";
import { FluidReleaseMachine } from "../machines/index.js";
import { getRunPolicyCheckDefault } from "../repoConfig.js";
import { StateMachineCommand } from "../stateMachineCommand.js";
Expand Down Expand Up @@ -90,10 +95,13 @@ export default class ReleaseCommand extends StateMachineCommand<typeof ReleaseCo
const releaseGroup = packageOrReleaseGroup.name;
const releaseVersion = packageOrReleaseGroup.version;

const currentBranch = await context.gitRepo.getCurrentBranchName();
const bumpType = await getBumpType(flags.bumpType, currentBranch, releaseVersion);

// eslint-disable-next-line no-warning-comments
// TODO: can be removed once server team owns server releases
// eslint-disable-next-line import/no-deprecated
if (flags.releaseGroup === MonoRepoKind.Server && flags.bumpType === "minor") {
if (flags.releaseGroup === MonoRepoKind.Server && bumpType === "minor") {
this.error(`Server release are always a ${chalk.bold("MAJOR")} release`);
}

Expand All @@ -117,7 +125,7 @@ export default class ReleaseCommand extends StateMachineCommand<typeof ReleaseCo
releaseVersion,
context,
promptWriter: new PromptWriter(logger),
bumpType: flags.bumpType as VersionBumpType,
bumpType,
versionScheme: detectVersionScheme(releaseVersion),
shouldSkipChecks: flags.skipChecks,
shouldCheckPolicy:
Expand All @@ -132,3 +140,46 @@ export default class ReleaseCommand extends StateMachineCommand<typeof ReleaseCo
};
}
}

/**
* Gets the bump type to use. If a bumpType was passed in, use it. Otherwise use the default for the branch. If
* there's no default for the branch, ask the user.
*/
async function getBumpType(
inputBumpType: VersionBumpType | undefined,
branch: string,
version: string,
): Promise<VersionBumpType> {
const bumpedMajor = bumpVersionScheme(version, "major");
const bumpedMinor = bumpVersionScheme(version, "minor");
const bumpedPatch = bumpVersionScheme(version, "patch");

let bumpType = inputBumpType ?? getDefaultBumpTypeForBranch(branch);
if (bumpType === undefined) {
const selectedBumpType = await rawlist({
message: `The current branch is '${branch}'. There is no default bump type for this branch. What type of release are you doing?`,
choices: [
{
value: "major" as VersionBumpType,
name: `major (${version} => ${bumpedMajor.version})`,
},
{
value: "minor" as VersionBumpType,
name: `minor (${version} => ${bumpedMinor.version})`,
},
{
value: "patch" as VersionBumpType,
name: `patch (${version} => ${bumpedPatch.version})`,
},
],
});

bumpType = selectedBumpType;
}

if (bumpType === undefined) {
throw new Error(`bumpType is undefined.`);
}

return bumpType;
}
117 changes: 117 additions & 0 deletions build-tools/packages/build-cli/src/handlers/checkFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/

import { strict as assert } from "node:assert";
import { existsSync } from "node:fs";

import { confirm, rawlist } from "@inquirer/prompts";
import execa from "execa";
import { Machine } from "jssm";
Expand All @@ -28,6 +30,12 @@ import { getRunPolicyCheckDefault } from "../repoConfig.js";
import { FluidReleaseStateHandlerData } from "./fluidReleaseStateHandler.js";
import { BaseStateHandler, StateHandlerFunction } from "./stateHandlers.js";

/**
* Only client and server release groups use changesets and the related release note and per-package changelog
* generation. Other release groups use various other means to track changes.
*/
const releaseGroupsUsingChangesets = new Set(["client", "server"]);

/**
* Checks that the current branch matches the expected branch for a release.
*
Expand Down Expand Up @@ -472,6 +480,115 @@ export const checkAssertTagging: StateHandlerFunction = async (
return true;
};

/**
* Checks that release notes have been generated.
*
* If release notes exist, then this function will send the "success" action to the state machine and return `true`. The
* state machine will transition to the appropriate state based on the "success" action.
*
* If release notes have not been generated, then this function will send the "failure" action to the state machine and
* still return `true`, since the state has been handled. The state machine will transition to the appropriate state
* based on the "failure" action.
*
* Once this function returns, the state machine's state will be reevaluated and passed to another state handler.
*
* @param state - The current state machine state.
* @param machine - The state machine.
* @param testMode - Set to true to run function in test mode. In test mode, the function returns true immediately.
* @param log - A logger that the function can use for logging.
* @param data - An object with handler-specific contextual data.
* @returns True if the state was handled; false otherwise.
*/
export const checkReleaseNotes: StateHandlerFunction = async (
state: MachineState,
machine: Machine<unknown>,
testMode: boolean,
log: CommandLogger,
data: FluidReleaseStateHandlerData,
): Promise<boolean> => {
if (testMode) return true;

const { bumpType, releaseGroup, releaseVersion } = data;

if (
// Only some release groups use changeset-based change-tracking.
releaseGroupsUsingChangesets.has(releaseGroup) &&
// This check should only be run for minor/major releases. Patch releases do not use changesets or generate release
// notes so there is no need to check them.
bumpType !== "patch"
) {
// Check if the release notes file exists
const filename = `RELEASE_NOTES/${releaseVersion}.md`;

if (!existsSync(filename)) {
log.logHr();
log.errorLog(
`Release notes for ${releaseGroup} version ${releaseVersion} are not found.`,
);
BaseStateHandler.signalFailure(machine, state);
return false;
}
}

BaseStateHandler.signalSuccess(machine, state);
return true;
};

/**
* Checks that changelogs have been generated.
*
* If changelogs have been generated for the current release, then this function will send the "success" action to the
* state machine and return `true`. The state machine will transition to the appropriate state based on the "success"
* action.
*
* If changelogs have not been generated, then this function will send the "failure" action to the state machine and
* still return `true`, since the state has been handled. The state machine will transition to the appropriate state
* based on the "failure" action.
*
* Once this function returns, the state machine's state will be reevaluated and passed to another state handler.
*
* @param state - The current state machine state.
* @param machine - The state machine.
* @param testMode - Set to true to run function in test mode. In test mode, the function returns true immediately.
* @param log - A logger that the function can use for logging.
* @param data - An object with handler-specific contextual data.
* @returns True if the state was handled; false otherwise.
*/
export const checkChangelogs: StateHandlerFunction = async (
state: MachineState,
machine: Machine<unknown>,
testMode: boolean,
log: CommandLogger,
data: FluidReleaseStateHandlerData,
): Promise<boolean> => {
if (testMode) return true;

const { releaseGroup, bumpType } = data;

if (
// Only some release groups use changeset-based change-tracking.
releaseGroupsUsingChangesets.has(releaseGroup) &&
// This check should only be run for minor/major releases. Patch releases do not use changesets or generate
// per-package changelogs so there is no need to check them.
bumpType !== "patch"
) {
const confirmed = await confirm({
message: "Did you generate and commit the CHANGELOG.md files for the release?",
});

if (confirmed !== true) {
log.logHr();
log.errorLog(`Changelogs must be generated.`);
BaseStateHandler.signalFailure(machine, state);
// State was handled, so return true.
return true;
}
}

BaseStateHandler.signalSuccess(machine, state);
return true;
};

/**
* Checks that a release branch exists.
*
Expand Down
Loading

0 comments on commit bffa806

Please sign in to comment.