From c0af0f62638ac0f445d079ca5237593510064484 Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Fri, 22 Dec 2023 16:21:26 +0000 Subject: [PATCH] fix(cli): keep comments in YAML frontmatter Switch the YAML library from [`js-yaml`][1] to [`yaml`][2]. This allows us to keep YAML comments in the frontmatter. [1]: https://www.npmjs.com/package/js-yaml [2]: https://www.npmjs.com/package/yaml --- packages/cli/package.json | 4 +- packages/cli/src/commander.test.ts | 7 +++ packages/cli/src/frontmatter.ts | 57 +++++++------------ .../cli/test/fixtures/connected-diagram.mmd | 1 + pnpm-lock.yaml | 34 ++++++----- 5 files changed, 49 insertions(+), 54 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 3e5bff1..e2a148e 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -59,11 +59,11 @@ "@inquirer/select": "^1.3.1", "@mermaidchart/sdk": "workspace:^", "commander": "^11.1.0", - "js-yaml": "^4.1.0", "remark": "^15.0.1", "remark-frontmatter": "^5.0.0", "remark-gfm": "^4.0.0", "to-vfile": "^8.0.0", - "unist-util-visit": "^5.0.0" + "unist-util-visit": "^5.0.0", + "yaml": "^2.3.4" } } diff --git a/packages/cli/src/commander.test.ts b/packages/cli/src/commander.test.ts index 2c5dc4d..9a1f797 100644 --- a/packages/cli/src/commander.test.ts +++ b/packages/cli/src/commander.test.ts @@ -435,6 +435,7 @@ describe('pull', () => { const mockedDiagram = { ...mockedEmptyDiagram, code: `--- +# comments in YAML shouldn't be removed title: My cool flowchart --- flowchart TD @@ -453,6 +454,7 @@ title: My cool flowchart expect(diagramContents).toContain( `id: https://test.mermaidchart.invalid/d/${mockedDiagram.documentID}`, ); + expect(diagramContents).toContain("# comments in YAML shouldn't be removed"); expect(diagramContents).toContain("flowchart TD\n A[I've been updated!]"); } }); @@ -521,6 +523,11 @@ describe('push', () => { code: expect.not.stringContaining('id:'), }), ); + expect(vi.mocked(MermaidChart.prototype.setDocument)).toHaveBeenCalledWith( + expect.objectContaining({ + code: expect.stringMatching(/^# This comment should be pushed to the server/m), + }), + ); }); it('should push documents from within markdown file', async () => { diff --git a/packages/cli/src/frontmatter.ts b/packages/cli/src/frontmatter.ts index 6cb6f02..eb72aaa 100644 --- a/packages/cli/src/frontmatter.ts +++ b/packages/cli/src/frontmatter.ts @@ -1,9 +1,7 @@ /** * Copied from https://github.com/mermaid-js/mermaid/blob/4a4e614b646bdb5f91f02d0483a7704b315d09fd/packages/mermaid/src/diagram-api/regexes.ts */ - -// The "* as yaml" part is necessary for tree-shaking -import * as yaml from 'js-yaml'; +import { parseDocument, type Document, YAMLMap, isMap } from 'yaml'; const frontMatterRegex = /^-{3}\s*[\n\r](.*?)[\n\r]-{3}\s*[\n\r]+/s; const urlIDRegex = /(?.*)\/d\/(?[\w-]+)/; @@ -59,20 +57,13 @@ function splitFrontMatter(text: string) { } } -function parseFrontMatterYAML(frontMatterYaml: string) { - let parsed: FrontMatterMetadata = - // TODO: replace with https://www.npmjs.com/package/yaml so that we can - // read/write comments too - yaml.load(frontMatterYaml, { - // To support config, we need JSON schema. - // https://www.yaml.org/spec/1.2/spec.html#id2803231 - schema: yaml.JSON_SCHEMA, - }) ?? {}; - - // To handle runtime data type changes - parsed = typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : {}; +function parseFrontMatterYAML(frontMatterYaml: string): Document { + const document: Document = parseDocument(frontMatterYaml); + if (!isMap(document.contents)) { + document.contents = new YAMLMap(); + } - return parsed; + return document as unknown as Document; } /** @@ -85,7 +76,7 @@ function parseFrontMatterYAML(frontMatterYaml: string) { export function extractFrontMatter(text: string): FrontMatterResult { const { diagramText, frontMatter } = splitFrontMatter(text); - const parsed = parseFrontMatterYAML(frontMatter); + const parsed = parseFrontMatterYAML(frontMatter).toJSON(); const metadata: FrontMatterMetadata = {}; @@ -116,16 +107,16 @@ export function extractFrontMatter(text: string): FrontMatterResult { * @param newMetadata - The metadata fields to update. * @returns The text with the updated YAML frontmatter. */ -export function injectFrontMatter(text: string, newMetadata: Partial) { +export function injectFrontMatter(text: string, newMetadata: Pick) { const { diagramText, frontMatter } = splitFrontMatter(text); - const parsed = parseFrontMatterYAML(frontMatter); + const document = parseFrontMatterYAML(frontMatter); - const mergedFrontmatter = { ...parsed, ...newMetadata }; + for (const [key, value] of Object.entries(newMetadata)) { + document.contents.set(key, value); + } - return `---\n${yaml.dump(mergedFrontmatter, { - schema: yaml.JSON_SCHEMA, - })}---\n${diagramText}`; + return `---\n${document.toString()}---\n${diagramText}`; } /** @@ -138,24 +129,16 @@ export function injectFrontMatter(text: string, newMetadata: Partial) { const { diagramText, frontMatter } = splitFrontMatter(text); - const parsedFrontMatter = parseFrontMatterYAML(frontMatter); + const document = parseFrontMatterYAML(frontMatter); - const entries = Object.entries(parsedFrontMatter) - .map((val) => { - if (keysToRemove.has(val[0] as keyof FrontMatterMetadata)) { - return null; - } else { - return val; - } - }) - .filter((val) => val) as [string, any][]; // eslint-disable-line @typescript-eslint/no-explicit-any + for (const key of keysToRemove) { + document.contents.delete(key); + } - if (entries.length === 0) { + if (document.contents.items.length === 0) { // skip creating frontmatter if there is no frontmatter return diagramText; } else { - return `---\n${yaml.dump(Object.fromEntries(entries), { - schema: yaml.JSON_SCHEMA, - })}---\n${diagramText}`; + return `---\n${document.toString()}---\n${diagramText}`; } } diff --git a/packages/cli/test/fixtures/connected-diagram.mmd b/packages/cli/test/fixtures/connected-diagram.mmd index 266c594..b3525c8 100644 --- a/packages/cli/test/fixtures/connected-diagram.mmd +++ b/packages/cli/test/fixtures/connected-diagram.mmd @@ -1,4 +1,5 @@ --- +# This comment should be pushed to the server title: My cool flowchart id: https://test.mermaidchart.invalid/d/my-test-document-id --- diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8dca37b..2a051e3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -89,9 +89,6 @@ importers: commander: specifier: ^11.1.0 version: 11.1.0 - js-yaml: - specifier: ^4.1.0 - version: 4.1.0 remark: specifier: ^15.0.1 version: 15.0.1 @@ -107,6 +104,9 @@ importers: unist-util-visit: specifier: ^5.0.0 version: 5.0.0 + yaml: + specifier: ^2.3.4 + version: 2.4.1 devDependencies: '@cspell/eslint-plugin': specifier: ^8.0.0 @@ -385,7 +385,7 @@ importers: version: 4.9.5 vite: specifier: ^4.3.7 - version: 4.3.7(@types/node@20.8.5) + version: 4.3.7(@types/node@18.18.12) vitest: specifier: ^0.30.1 version: 0.30.1(jsdom@21.0.0) @@ -3767,7 +3767,7 @@ packages: open: 8.4.2 tree-kill: 1.2.2 underscore: 1.13.6 - yaml: 2.3.2 + yaml: 2.4.1 yargs: 17.7.2 optionalDependencies: keytar: 7.9.0 @@ -3840,7 +3840,7 @@ packages: uuid: 8.3.2 validator: 13.11.0 xml2js: 0.5.0 - yaml: 2.3.2 + yaml: 2.4.1 transitivePeerDependencies: - bluebird - debug @@ -4182,7 +4182,7 @@ packages: svelte: 3.59.1 tiny-glob: 0.2.9 undici: 5.22.1 - vite: 4.3.7(@types/node@20.8.5) + vite: 4.3.7(@types/node@18.18.12) transitivePeerDependencies: - supports-color @@ -4197,7 +4197,7 @@ packages: '@sveltejs/vite-plugin-svelte': 2.4.5(svelte@3.59.1)(vite@4.3.7) debug: 4.3.4 svelte: 3.59.1 - vite: 4.3.7(@types/node@20.8.5) + vite: 4.3.7(@types/node@18.18.12) transitivePeerDependencies: - supports-color @@ -4215,7 +4215,7 @@ packages: magic-string: 0.30.3 svelte: 3.59.1 svelte-hmr: 0.15.3(svelte@3.59.1) - vite: 4.3.7(@types/node@20.8.5) + vite: 4.3.7(@types/node@18.18.12) vitefu: 0.2.4(vite@4.3.7) transitivePeerDependencies: - supports-color @@ -4386,6 +4386,7 @@ packages: resolution: {integrity: sha512-SPlobFgbidfIeOYlzXiEjSYeIJiOCthv+9tSQVpvk4PAdIIc+2SmjNVzWXk9t0Y7dl73Zdf+OgXKHX9XtkqUpw==} dependencies: undici-types: 5.25.3 + dev: true /@types/normalize-package-data@2.4.1: resolution: {integrity: sha512-Gj7cI7z+98M282Tqmp2K5EIsoouUEzbBJhQQzDE3jSIRk6r9gsz0oUokqIUR4u1R3dMHo0pDHM7sNOHyhulypw==} @@ -5311,6 +5312,7 @@ packages: /argparse@2.0.1: resolution: {integrity: sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==} + dev: true /array-buffer-byte-length@1.0.0: resolution: {integrity: sha512-LPuwb2P+NrQw3XhxGc36+XSvuBPopovXYTR9Ew++Du9Yb/bx5AzBfrIsBoj0EZUifjQU+sHL21sseZ3jerWO/A==} @@ -9685,6 +9687,7 @@ packages: hasBin: true dependencies: argparse: 2.0.1 + dev: true /js@0.1.0: resolution: {integrity: sha512-ZBbGYOpact8QAH9RprFWL4RAESYwbDodxiuDjOnzwzzk9pBzKycoifGuUrHHcDixE/eLMKPHRaXenTgu1qXBqA==} @@ -12107,7 +12110,7 @@ packages: dependencies: lilconfig: 2.1.0 postcss: 8.4.14 - yaml: 2.3.1 + yaml: 2.4.1 dev: true /postcss-nested@6.0.0(postcss@8.4.14): @@ -14057,6 +14060,7 @@ packages: /undici-types@5.25.3: resolution: {integrity: sha512-Ga1jfYwRn7+cP9v8auvEXN1rX3sWqlayd4HP7OKk4mZWylEmu3KzXDUGrQUN6Ol7qo1gPvB2e5gX6udnyEPgdA==} + dev: true /undici-types@5.26.5: resolution: {integrity: sha512-JlCMO+ehdEIKqlFxk6IfVoAUVmgz7cU7zD/h9XZ0qzeosSHmUJVOzSQvvYSYWXkFXC+IfLKSIffhv0sVZup6pA==} @@ -14418,7 +14422,6 @@ packages: rollup: 3.26.3 optionalDependencies: fsevents: 2.3.3 - dev: true /vite@4.3.7(@types/node@20.8.5): resolution: {integrity: sha512-MTIFpbIm9v7Hh5b0wSBgkcWzSBz7SAa6K/cBTwS4kUiQJfQLFlZZRJRQgqunCVzhTPCk674tW+0Qaqh3Q00dBg==} @@ -14451,6 +14454,7 @@ packages: rollup: 3.26.3 optionalDependencies: fsevents: 2.3.3 + dev: true /vitefu@0.2.4(vite@4.3.7): resolution: {integrity: sha512-fanAXjSaf9xXtOOeno8wZXIhgia+CZury481LsDaV++lSvcU2R9Ch2bPh3PYFyoHW+w9LqAeYRISVQjUIew14g==} @@ -14460,7 +14464,7 @@ packages: vite: optional: true dependencies: - vite: 4.3.7(@types/node@20.8.5) + vite: 4.3.7(@types/node@18.18.12) /vitest@0.30.1(jsdom@21.0.0): resolution: {integrity: sha512-y35WTrSTlTxfMLttgQk4rHcaDkbHQwDP++SNwPb+7H8yb13Q3cu2EixrtHzF27iZ8v0XCciSsLg00RkPAzB/aA==} @@ -14985,10 +14989,10 @@ packages: engines: {node: '>= 14'} dev: true - /yaml@2.3.2: - resolution: {integrity: sha512-N/lyzTPaJasoDmfV7YTrYCI0G/3ivm/9wdG0aHuheKowWQwGTsK0Eoiw6utmzAnI6pkJa0DUVygvp3spqqEKXg==} + /yaml@2.4.1: + resolution: {integrity: sha512-pIXzoImaqmfOrL7teGUBt/T7ZDnyeGBWyXQBvOVhLkWLN37GXv8NMLK406UY6dS51JfcQHsmcW5cJ441bHg6Lg==} engines: {node: '>= 14'} - dev: true + hasBin: true /yargs-parser@21.1.1: resolution: {integrity: sha512-tVpsJW7DdjecAiFpbIB1e3qxIQsE6NoPc5/eTdrbbIC4h0LVsWhnoa3g+m2HclBIujHzsxZ4VJVA+GUuc2/LBw==}