From b989ff536216521f7d2e46dc719b2e9d48ccb154 Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Sun, 2 Jul 2023 23:44:19 +0100 Subject: [PATCH 1/3] fix: change sankey config types to be unions Replace the TypeScript `enum {a = "a", b = "b"}` types with TypeScript's literal types (e.g. `"a" | "b"`). This is because TypeScript enums are [_not_ a type-level addition to JavaScript][1], and even the official TypeScript docs say to be careful when using. [1]: https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#enums --- packages/mermaid/src/config.type.ts | 14 ++----------- packages/mermaid/src/defaultConfig.ts | 6 +++--- .../src/diagrams/sankey/sankeyRenderer.ts | 20 +++++++++---------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/packages/mermaid/src/config.type.ts b/packages/mermaid/src/config.type.ts index 4d40b33c8..138eee44f 100644 --- a/packages/mermaid/src/config.type.ts +++ b/packages/mermaid/src/config.type.ts @@ -412,18 +412,8 @@ export interface FlowchartDiagramConfig extends BaseDiagramConfig { wrappingWidth?: number; } -export enum SankeyLinkColor { - source = 'source', - target = 'target', - gradient = 'gradient', -} - -export enum SankeyNodeAlignment { - left = 'left', - right = 'right', - center = 'center', - justify = 'justify', -} +export type SankeyLinkColor = 'source' | 'target' | 'gradient'; +export type SankeyNodeAlignment = 'left' | 'right' | 'center' | 'justify'; export interface SankeyDiagramConfig extends BaseDiagramConfig { width?: number; diff --git a/packages/mermaid/src/defaultConfig.ts b/packages/mermaid/src/defaultConfig.ts index 6e24b6391..488493030 100644 --- a/packages/mermaid/src/defaultConfig.ts +++ b/packages/mermaid/src/defaultConfig.ts @@ -1,5 +1,5 @@ import theme from './themes/index.js'; -import { MermaidConfig, SankeyLinkColor, SankeyNodeAlignment } from './config.type.js'; +import { type MermaidConfig } from './config.type.js'; /** * **Configuration methods in Mermaid version 8.6.0 have been updated, to learn more[[click * here](8.6.0_docs.md)].** @@ -2273,8 +2273,8 @@ const config: Partial = { sankey: { width: 800, height: 400, - linkColor: SankeyLinkColor.gradient, - nodeAlignment: SankeyNodeAlignment.justify, + linkColor: 'gradient', + nodeAlignment: 'justify', useMaxWidth: false, }, fontSize: 16, diff --git a/packages/mermaid/src/diagrams/sankey/sankeyRenderer.ts b/packages/mermaid/src/diagrams/sankey/sankeyRenderer.ts index e6e18f113..a9ee698e9 100644 --- a/packages/mermaid/src/diagrams/sankey/sankeyRenderer.ts +++ b/packages/mermaid/src/diagrams/sankey/sankeyRenderer.ts @@ -18,17 +18,17 @@ import { } from 'd3-sankey'; import { configureSvgSize } from '../../setupGraphViewbox.js'; import { Uid } from '../../rendering-util/uid.js'; -import { SankeyLinkColor, SankeyNodeAlignment } from '../../config.type.js'; +import type { SankeyLinkColor, SankeyNodeAlignment } from '../../config.type.js'; // Map config options to alignment functions const alignmentsMap: Record< SankeyNodeAlignment, (node: d3SankeyNode, n: number) => number > = { - [SankeyNodeAlignment.left]: d3SankeyLeft, - [SankeyNodeAlignment.right]: d3SankeyRight, - [SankeyNodeAlignment.center]: d3SankeyCenter, - [SankeyNodeAlignment.justify]: d3SankeyJustify, + left: d3SankeyLeft, + right: d3SankeyRight, + center: d3SankeyCenter, + justify: d3SankeyJustify, }; /** @@ -157,9 +157,9 @@ export const draw = function (text: string, id: string, _version: string, diagOb .attr('class', 'link') .style('mix-blend-mode', 'multiply'); - const linkColor = conf?.linkColor || SankeyLinkColor.gradient; + const linkColor = conf?.linkColor || 'gradient'; - if (linkColor === SankeyLinkColor.gradient) { + if (linkColor === 'gradient') { const gradient = link .append('linearGradient') .attr('id', (d: any) => (d.uid = Uid.next('linearGradient-')).id) @@ -180,13 +180,13 @@ export const draw = function (text: string, id: string, _version: string, diagOb let coloring: any; switch (linkColor) { - case SankeyLinkColor.gradient: + case 'gradient': coloring = (d: any) => d.uid; break; - case SankeyLinkColor.source: + case 'source': coloring = (d: any) => colorScheme(d.source.id); break; - case SankeyLinkColor.target: + case 'target': coloring = (d: any) => colorScheme(d.target.id); break; default: From fbf79ffcc01314c0883bfc77cd5dc19b43ac968d Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Sun, 2 Jul 2023 23:56:42 +0100 Subject: [PATCH 2/3] refactor: replace TypeScript enum with JS obj [`const` assertions where added in TypeScript 3.4][1] and can be used to create enum-like objects in plain JavaScript, that act like TypeScript enums, but has none of the downsides of TypeScript enums. [1]: https://devblogs.microsoft.com/typescript/announcing-typescript-3-4/#const-assertions --- packages/mermaid/src/config.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/mermaid/src/config.ts b/packages/mermaid/src/config.ts index 838716e2f..e00663827 100644 --- a/packages/mermaid/src/config.ts +++ b/packages/mermaid/src/config.ts @@ -226,9 +226,11 @@ export const reset = (config = siteConfig): void => { updateCurrentConfig(config, directives); }; -enum ConfigWarning { - 'LAZY_LOAD_DEPRECATED' = 'The configuration options lazyLoadedDiagrams and loadExternalDiagramsAtStartup are deprecated. Please use registerExternalDiagrams instead.', -} +const ConfigWarning = { + LAZY_LOAD_DEPRECATED: + 'The configuration options lazyLoadedDiagrams and loadExternalDiagramsAtStartup are deprecated. Please use registerExternalDiagrams instead.', +} as const; + type ConfigWarningStrings = keyof typeof ConfigWarning; const issuedWarnings: { [key in ConfigWarningStrings]?: boolean } = {}; const issueWarning = (warning: ConfigWarningStrings) => { From 79688a1dc10139c7d751c270645dd368f85f3d38 Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Mon, 3 Jul 2023 00:04:16 +0100 Subject: [PATCH 3/3] build: forbid using TypeScript enums using eslint [TypeScript enums][1] are an unusual TypeScript feature, because it's one of the only features that is "not a type-level extension of JavaScript". This means TypeScript generates custom code for enums, which can cause a bunch of issues, especially as this custom code can be built differently depending on which bundler you use, and because of this, many people discourage the use of enums: - [The Dangers of TypeScript Enums][2] - [Tidy TypeScript: Prefer union types over enums][3] - [TypeScript: Handbook - Enums # Objects vs Enums][4] I've added an ESLint rule that forbids TypeScript enums, as in most cases, it's better to use string literals instead, e.g. like `type a = "a" | "b" | "c";`. [1]: https://www.typescriptlang.org/docs/handbook/enums.html#string-enums [2]: https://dev.to/azure/the-dangers-of-typescript-enums-55pd [3]: https://fettblog.eu/tidy-typescript-avoid-enums/ [4]: https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums --- .eslintrc.cjs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index e6f99a8bf..342d15d9f 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -123,6 +123,14 @@ module.exports = { files: ['*.{ts,tsx}'], plugins: ['tsdoc'], rules: { + 'no-restricted-syntax': [ + 'error', + { + selector: 'TSEnumDeclaration', + message: + 'Prefer using TypeScript union types over TypeScript enum, since TypeScript enums have a bunch of issues, see https://dev.to/dvddpl/whats-the-problem-with-typescript-enums-2okj', + }, + ], 'tsdoc/syntax': 'error', }, },