BREAKING CHANGE: string literal types in favour of const enums

[Sayan751]

Pull Request

📖 Description

This PR introduces a BREAKING CHANGE, as it converts the following exported const enums to string literal types, as those are more developer friendly, and does not suffers from the issue outlined in #1736.

• @aurelia/dialog
  • DialogDeactivationStatuses

    - const enum DialogDeactivationStatuses {
    -   Ok = 'ok',
    -   Error = 'error',
    -   Cancel = 'cancel',
    -   Abort = 'abort',
    - }
    +  type DialogDeactivationStatuses = 'ok' | 'error' | 'cancel' | 'abort';

• @aurelia/kernel
  • ColorOptions

  - const enum ColorOptions {
  -   noColors = 0,
  -   colors = 1,
  - }
  + type ColorOptions = 'no-colors' | 'colors';

• @aurelia/platform
  • TaskStatus

  - const enum TaskStatus {
  -   pending = 0,
  -   running = 1,
  -   completed = 2,
  -   canceled = 3,
  - }
  + type TaskStatus = 'pending' | 'running' | 'completed' | 'canceled';

• @aurelia/router-lite
  • ExpressionKind

  - const enum ExpressionKind {
  -   Route,
  -   CompositeSegment,
  -   ScopedSegment,
  -   SegmentGroup,
  -   Segment,
  -   Component,
  -   Action,
  -   Viewport,
  -   ParameterList,
  -   Parameter,
  - }
  + type ExpressionKind = 'Route' | 'CompositeSegment' | 'ScopedSegment' | 'SegmentGroup' | 'Segment' | 'Component' | 'Action' | 'Viewport' | 'ParameterList' | 'Parameter';

• @aurelia/router
  • ReloadBehavior

  - const enum ReloadBehavior {
  -   default = 'default',
  -   disallow = 'disallow',
  -   reload = 'reload',
  -   refresh = 'refresh',
  - }
  + type ReloadBehavior = 'default' | 'disallow' | 'reload' | 'refresh';

• @aurelia/runtime-html
  • CommandType

  - const enum CommandType {
  -   None       = 0b0_000,
  -   IgnoreAttr = 0b0_001,
  - }
  + type CommandType = 'None' | 'IgnoreAttr';

  • DefinitionType

  - const enum DefinitionType {
  -   Element = 1,
  -   Attribute = 2,
  - }
  + type DefinitionType = 'Element' | 'Attribute';

  • ViewModelKind

  - const enum ViewModelKind {
  -   customElement,
  -   customAttribute,
  -   synthetic
  - }
  + type ViewModelKind = 'customElement' |  'customAttribute' |  'synthetic';

• @aurelia/runtime
  • ExpressionKind

  - const enum ExpressionKind {
  -   AccessThis,
  -   AccessGlobal,
  -   AccessScope,
  -   ArrayLiteral,
  -   ObjectLiteral,
  -   PrimitiveLiteral,
  -   Template,
  -   Unary,
  -   CallScope,
  -   CallMember,
  -   CallFunction,
  -   CallGlobal,
  -   AccessMember,
  -   AccessKeyed,
  -   TaggedTemplate,
  -   Binary,
  -   Conditional,
  -   Assign,
  -   ArrowFunction,
  -   ValueConverter,
  -   BindingBehavior,
  -   ArrayBindingPattern,
  -   ObjectBindingPattern,
  -   BindingIdentifier,
  -   ForOfStatement,
  -   Interpolation,
  -   ArrayDestructuring,
  -   ObjectDestructuring,
  -   DestructuringAssignmentLeaf,
  -   DestructuringAssignmentRestLeaf,
  -   Custom,
  - }
  + type ExpressionKind = 'AccessThis' | 'AccessGlobal' | 'AccessScope' | 'ArrayLiteral' | 'ObjectLiteral' | 'PrimitiveLiteral' | 'Template' | 'Unary' | 'CallScope' | 'CallMember' | 'CallFunction' | 'CallGlobal' | 'AccessMember' | 'AccessKeyed' | 'TaggedTemplate' | 'Binary' | 'Conditional' | 'Assign' | 'ArrowFunction' | 'ValueConverter' | 'BindingBehavior' | 'ArrayBindingPattern' | 'ObjectBindingPattern' | 'BindingIdentifier' | 'ForOfStatement' | 'Interpolation' | 'ArrayDestructuring' | 'ObjectDestructuring' | 'DestructuringAssignmentLeaf' | 'DestructuringAssignmentRestLeaf' | 'Custom';

  • ExpressionType

  - const enum ExpressionType {
  -           None = 0,
  -  Interpolation = 0b0_000001,
  -     IsIterator = 0b0_000010,
  -    IsChainable = 0b0_000100,
  -     IsFunction = 0b0_001000,
  -     IsProperty = 0b0_010000,
  -     IsCustom   = 0b0_100000,
  - }
  + type ExpressionType = 'None' | 'Interpolation' | 'IsIterator' | 'IsChainable' | 'IsFunction' | 'IsProperty' | 'IsCustom';

  • CollectionKind

  - const enum CollectionKind {
  -   indexed = 0b1000,
  -   keyed   = 0b0100,
  -   array   = 0b1001,
  -   map     = 0b0110,
  -   set     = 0b0111,
  - }
  + type CollectionKind = 'indexed' | 'keyed' | 'array' | 'map' | 'set';

• @aurelia/validation-html
  • ValidateEventKind

  - const enum ValidateEventKind {
  -   validate = 'validate',
  -   reset = 'reset',
  - }
  + type ValidateEventKind = 'validate' | 'reset';

🎫 Issues

Closes #1736.

👩‍💻 Reviewer Notes

There are still const enums in code. These fall under the following categories:

• Used internally; not for public consumption.
• Flags enum, and hence a string literal type won't make sense.
• Used widely, and hence a replacement will cause larger bundle size.
• Ordinal enums like LogLevel from @aurelia/kernel (LogLevel.warn < LogLevel.error), where the string literal type would make the comparison needlessly difficult.

Here is a list of publicly consumable const enums:

• @aurelia/kernel: LogLevel
• @aurelia/runtime: AccessorType
• @aurelia/runtime-html: InstructionType, BindingMode, State

The docs have been updated to include information for TypeScript users.

📑 Test Plan

⏭ Next Steps

[codecov]

Codecov Report

Attention: 36 lines in your changes are missing coverage. Please review.

Comparison is base (71f82cf) 88.78% compared to head (5cd4e18) 88.78%.

❗ Current head 5cd4e18 differs from pull request most recent head 8b08369. Consider uploading reports for the commit 8b08369 to get more accurate results

Files	Patch %	Lines
packages/runtime-html/src/templating/controller.ts	74.28%	9 Missing ⚠️
packages/runtime/src/binding/ast.visitor.ts	78.12%	7 Missing ⚠️
packages/router-lite/src/route-expression.ts	33.33%	6 Missing ⚠️
packages/runtime/src/binding/ast.eval.ts	90.00%	5 Missing ⚠️
packages/platform/src/index.ts	81.81%	4 Missing ⚠️
.../router/src/instructions/instruction-parameters.ts	81.81%	2 Missing ⚠️
packages/compat-v1/src/compat-delegate.ts	50.00%	1 Missing ⚠️
packages/i18n/src/t/translation-binding.ts	50.00%	1 Missing ⚠️
packages/state/src/state-templating.ts	75.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1870   +/-   ##
=======================================
  Coverage   88.78%   88.78%           
=======================================
  Files         248      247    -1     
  Lines       22349    22307   -42     
  Branches     5205     5192   -13     
=======================================
- Hits        19842    19806   -36     
+ Misses       2507     2501    -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bigopon]

There may be enums that feel uncomfortable to express in string, as they used bits to express multiple characteristics. But probably we can try to ignore that since atm, its only the binding command type.
Ill do a simple check for size based on the examples/helloworld app.

[bigopon]

Good work 👍
Quite a tedious PR thanks @Sayan751
For bundle size, it's actually few hundred bytes smaller for a helloworld app 😄

[bigopon]

Maybe eventually we won't use const enum. Applications shouldn't be restricted by what we do. The cost isn't too high.

[bigopon]

I think we can convert binding mode to numbers & type as well instead of enum.

const toView = 1;
const ...

/**
 * Comment explaining the modes here
 */
export type BindingMode =
  | typeof toView
  | typeof ...

It'll be fine I think as the places that use BindingMode are limited, and normally devs know what to be the values with literal primitive type easily if we have enough comments like this:

export interface BindableDefinition {
  /**
   * Some more comments here if really necessary <<<<<<<
   */
  mode: BindingMode;
}

We can also export a literal BindingMode object for convenience:

export const BindingMode = {
  toView = ...
} as const;

So it'll be just as good compared to the enum.

[bigopon]

@Sayan751 I've merged it for now. For the comments, please address them in a follow up PRs, if you prefer to have me handle them, pls let me know. Nice work 👍

[Sayan751]

I think we can convert binding mode to numbers & type as well instead of enum.

const toView = 1;
const ...

/**
 * Comment explaining the modes here
 */
export type BindingMode =
  | typeof toView
  | typeof ...

It'll be fine I think as the places that use BindingMode are limited, and normally devs know what to be the values with literal primitive type easily if we have enough comments like this:

export interface BindableDefinition {
  /**
   * Some more comments here if really necessary <<<<<<<
   */
  mode: BindingMode;
}

We can also export a literal BindingMode object for convenience:

export const BindingMode = {
  toView = ...
} as const;

So it'll be just as good compared to the enum.

This is a good idea. We can do this.

@Sayan751 I've merged it for now. For the comments, please address them in a follow up PRs, if you prefer to have me handle them, pls let me know. Nice work 👍

I will do a follow-up PR.