rushstack
rushstack copied to clipboard
[rush] Convert most enums to constants.
@elliot-nelson
Summary
This PR converts most enums in the rush-lib project to object constants with a merged type. This improves typesafety. For example:
import { BumpType } from '@microsoft/rush-lib';
const x: BumpType = BumpType.minor; // Works the same
const y: BumpType = 1234; // Used to be allowed, now errors
Issues
@octogonz - API Extractor isn't happy with the merged declarations. It has the following errors:
[api-extractor] src\api\EnvironmentConfiguration.ts:22:14 - (ae-different-release-tags) This symbol has another declaration with a different release tag
[api-extractor] src\api\EnvironmentConfiguration.ts:22:14 - (ae-missing-release-tag) "EnvironmentVariableNames" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
[api-extractor] src\api\EventHooks.ts:12:14 - (ae-different-release-tags) This symbol has another declaration with a different release tag
[api-extractor] src\api\EventHooks.ts:12:14 - (ae-missing-release-tag) "Event" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
[api-extractor] src\api\PackageJsonEditor.ts:13:14 - (ae-missing-release-tag) "DependencyType" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
[api-extractor] src\api\VersionPolicy.ts:26:14 - (ae-missing-release-tag) "BumpType" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
[api-extractor] src\api\VersionPolicy.ts:59:14 - (ae-missing-release-tag) "VersionPolicyDefinitionName" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
[api-extractor] src\logic\operations\OperationStatus.ts:9:14 - (ae-different-release-tags) This symbol has another declaration with a different release tag
[api-extractor] src\logic\operations\OperationStatus.ts:9:14 - (ae-missing-release-tag) "OperationStatus" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)
How it was tested
This PR still needs to be tested.
Looks good so far!
Played with the approach a bit in TS playground and it is doing what I want, specifically: it behaves rationally when interacting with supersets and subsets, like if I pass a BumpType into a function expecting 'major' | 'minor' | 'patch', I get the VSCode error Type "prerelease" is not assignable to type "BumpType", which is exactly what I want.
An advantage to this construct over a traditional union string is that if you add /** */ comments above each const property they show up on hover, which is nice.
EDIT: It looks like there's two different approaches you tried it; the const Foo = { } as const; is the one I think works better (the other one requires duplication of all property names afaict).
API Extractor isn't happy with the merged declarations. It has the following errors:
I'm not sure this new syntax is an improvement. We should discuss it.
API Extractor isn't happy with the merged declarations. It has the following errors:
I'm not sure this new syntax is an improvement. We should discuss it.
This syntax allows callers to continue to work as-is with the same type-safety as before (actually better, for any previously numeric enum), without depending on the TypeScript-specific "enum" feature. It's what we use internally as a drop-in replacement for const enum.
If targeting webpack 5, the optimal syntax gets a bit more complicated, because it becomes best to define each enum in its own file as a set of individual export const statements, then do:
import * as RawFooEnum from './FooEnum';
const FooEnum: typeof RawFooEnum = RawFooEnum;
type FooEnum = typeof RawFooEnum[keyof typeof RawFooEnum];
export { FooEnum };
This allows webpack to tree-shake out unused enum values.
Closing this for now: this can be revisited in the future.