FDC3 icon indicating copy to clipboard operation
FDC3 copied to clipboard

Refactoring ContextTypes to union type (instead of enum)

Open andreifloricel opened this issue 2 years ago • 1 comments

Currently the standard FDC3 Context types are defined as an enum: https://github.com/finos/FDC3/blob/43ce7b2c17b80b9117a339950789867641ea0929/src/context/ContextType.ts#L5-L25

This causes problems when constructing mapped and/or conditional types, e.g. generically typing a function so that for a given ContextType the corresponding Context structure is returned (fdc3.chart'` -> `Chart`, fdc3.action'->Action`, etc.).

The ContextType defined here is not helping, as it's NOT a valid union type: https://github.com/finos/FDC3/blob/43ce7b2c17b80b9117a339950789867641ea0929/src/context/ContextType.ts#L27

Also, in current versions of TypeScript everything enum types offer can be achieved with union types (and much more) without the inherent problems that TypeScript Enums have: https://www.typescriptlang.org/docs/handbook/enums.html#const-enum-pitfalls

My suggestion is to replace the current implementation with a simple union type. Possibly also clearly differentiate between standard FDC3 context types and custom ones:

export type StandardContextTypes =
  | 'fdc3.action'
  | 'fdc3.chart'
  | 'fdc3.chat.initSettings'
  | 'fdc3.chat.message'
  | 'fdc3.chat.room'
  | 'fdc3.chat.searchCriteria'
  | 'fdc3.contact'
  | 'fdc3.contactList'
  | 'fdc3.country'
  | 'fdc3.currency'
  | 'fdc3.email'
  | 'fdc3.instrument'
  | 'fdc3.instrumentList'
  | 'fdc3.interaction'
  | 'fdc3.message'
  | 'fdc3.organization'
  | 'fdc3.portfolio'
  | 'fdc3.position'
  | 'fdc3.nothing'
  | 'fdc3.timerange'
  | 'fdc3.transactionResult'
  | 'fdc3.valuation';

export type ContextTypes = StandardContextTypes | string;

Currently, I need to define these types in all applications that interact with FDC3, but it would be nice to have them in the main library.

andreifloricel avatar Dec 22 '23 18:12 andreifloricel

I've had similar trouble using the TS enums and migrated everything else (in generated code) to string unions recently. Hence, I support this. We had one question raised about whether this should even be in the NPM module at all (i.e. whether its actually useful at runtime). An alternative might be to make this an array that can more easily be used for a runtime check... See https://stackoverflow.com/questions/36836011/checking-validity-of-string-literal-union-type-at-runtime

If we make this change, we should probably advise on use. Perhaps even include a utility function that checks whether a context is a standard type in the API's Methods.ts or in the ContextType.ts file...

kriswest avatar Jan 30 '24 11:01 kriswest