From ac4ff96287f904c1e3c262bce4b389ccd37c7f73 Mon Sep 17 00:00:00 2001 From: "ai@codebar" Date: Sat, 8 Aug 2026 08:26:59 +0200 Subject: [PATCH] =?UTF-8?q?1.19.0=20=E2=80=94=20a=20real=20yaml=20mode,=20?= =?UTF-8?q?and=20the=20two=20diagnostics=20that=20were=20compiled=20away?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the app that adopted 1.18.0. Three acted on, one declined. CodeEditor loaded the JSON grammar for anything that was not `markdown`, so a consuming app's compiled-flow-definition page highlighted YAML through a JSON parser for the life of the page. It survived because the wrong grammar does not fail loudly: JSON still paints nine tokens in a typical flow definition — every `:` as punctuation, every bare integer as a number — which reads as syntax highlighting from across the room. What it cannot paint is `#`, a comment in YAML and nothing at all in JSON, so the Yaml story asserts on that line and fails when the document is served the JSON grammar. A "some token is coloured" assertion passes on the broken version, which is why there wasn't one. `yaml` is now a mode on both code surfaces (@codemirror/lang-yaml, a new optional peer — see the changelog's upgrade note, it is the one thing this release asks of a consumer), and an unknown language renders unhighlighted and warns instead of silently reaching for JSON. A component that quietly picks the wrong grammar is worse than one that refuses the mode: the first is indistinguishable from working. DataTable.rowKey has always been required with no default, and nothing said so at runtime — nine call sites shipped without it and keyed every row `undefined`. Vue's own "Missing required prop" warning cannot fire for any prop in this package: the library is compiled with isProd, @vue/compiler-sfc emits `required`/`type` only for a development build, and dist declares the prop as the bare `rowKey: {}`. Rendering DataTable with no rowKey against a development Vue produces no Vue warning at all. Same shape as the import.meta.env.DEV problem in helpers/dev.ts, so the check is written out by hand like the tone deprecation, and verified against the built bundle rather than src. CodeEditorProps.modelValue becomes `string | null`, which is what the runtime has always accepted (`?? ''` guards every read). Not `string | number | null` like Input: that is a native input whose value the DOM stringifies anyway, while this holds a document. Textarea, the closer sibling, is `string | null`. Declined: narrowing DataTableColumn.key to `keyof T & string`. It would forbid action and computed columns, it disagrees with SortState.key which is a free string on the wire, and for the row shapes it was proposed for it is the identity — DataTable is `T extends Record`, an interface only satisfies that by declaring the index signature, and once `[key: string]: unknown` is present `keyof T & string` IS `string` and `T[K]` IS `unknown`. The obstacle is the constraint, not the key. Reasoning in full in the changelog. Verified against a consumer fixture resolving the package through its exports map: vue-tsc accepts `language="yaml"` and a nullable v-model with no `?? ''`, rejects an unknown mode, and the built bundle warns for a missing rowKey under a development Vue while a supplied one stays silent. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 149 ++++++++++++++++++ package-lock.json | 33 +++- package.json | 15 +- .../organisms/CodeEditor.stories.ts | 90 +++++++++++ src/components/organisms/CodeEditor.vue | 55 ++++++- .../organisms/CodePreview.stories.ts | 18 +++ src/components/organisms/CodePreview.vue | 42 +++-- src/components/organisms/DataTable.vue | 29 +++- vite.config.ts | 1 + 9 files changed, 404 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ae7f08..06072a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,155 @@ All notable changes to `@codebar-ag/storybook`. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## v1.19.0 + +Four findings from the app that adopted 1.18.0, three of them acted on and one +declined. One is a rendering bug that had been shipping for the life of the +component: `CodeEditor` had no `yaml` mode and highlighted YAML as JSON without +saying so. + +**This release adds a peer dependency.** `@codemirror/lang-yaml` — see +[Upgrade notes](#upgrade-notes). Nothing else here requires a change to a +call site. + +### Added + +- **`CodeEditor` and `CodePreview` accept `language="yaml"`**, parsed by + `@codemirror/lang-yaml` and highlighted through the same shared theme as + every other code surface in the kit. + + The editor previously loaded the JSON grammar for **anything that was not + `markdown`**, so a consuming app's compiled-flow-definition page had been + highlighting YAML through a JSON parser since the day it was written. The + reason nobody noticed is worth recording, because it is what makes this class + of bug expensive: the wrong grammar does not fail loudly on YAML. Run the + JSON parser over a typical flow definition and it still paints nine tokens — + every `:` as punctuation, every bare integer as a number — which reads as + syntax highlighting from across the room. What it cannot paint is `#`, a + comment in YAML and nothing at all in JSON. The `Yaml` story asserts on that + line specifically, and fails if the document is served the JSON grammar; a + "some token is coloured" assertion passes on the broken version. + + `CodePreview` gains the mode too. A kit that can edit YAML but only preview + it as plain text is a difference no caller can explain. + +- **An unknown `language` now renders unhighlighted and says so**, instead of + silently falling back to JSON. Both components warn once per unknown value in + development, through the same `warnOnce` path as every other dev diagnostic + here. A component that quietly picks the wrong grammar is worse than one that + refuses the mode: the first is indistinguishable from working. + + `language` is a typed union, so the only callers who can reach that arm are + untyped ones — which is exactly who has no compiler to tell them. + +- **`DataTable` warns in development when `rowKey` is missing.** It has always + been a required prop with no default, and until now nothing said so at + runtime: nine call sites in one consuming app shipped without it, so every + row in those tables was keyed `undefined` and Vue could not tell any row from + any other. + + Vue's own "Missing required prop" warning cannot fire for **any** prop in + this package, which is why the years passed quietly. The library is compiled + with `isProd: true`, and `@vue/compiler-sfc` emits `type` and `required` only + for a development build — the published bundle declares the prop as the bare + `rowKey: {}`. Rendering `` with no `rowKey` against a development + build of Vue produces no Vue warning at all. This is the same shape as the + `import.meta.env.DEV` problem documented in `src/helpers/dev.ts`: a + diagnostic that reads as working and is compiled away before it reaches + anyone. The check is therefore written out by hand, like the tone + deprecation, and verified against the built bundle rather than against `src`. + + Every other required prop in this kit is unvalidated at runtime for the same + reason. Only `rowKey` is covered here, because only `rowKey` has evidence + attached; a general fix means compiling the library in development mode, and + that is its own release. + +### Changed + +- **`CodeEditorProps.modelValue` is `string | null`.** The runtime has always + accepted null — `?? ''` guards every read of it, and an empty document is + what it renders — but the type said `string`, so callers holding a nullable + column (a definition that has not been compiled yet, an optional description) + coerced with `?? ''` at the call site for a default the component already + applies. Same shape as `BreadcrumbItem.href` in 1.18.0: the type was narrower + than the behaviour. + + Deliberately **not** widened to `string | number | null`, which is what + `Input` takes. `Input` is a native `` whose `value` the DOM stringifies + anyway; `CodeEditor` holds a *document*, and a number would have to be + silently stringified into one on the way in and handed back as a string on + the way out. `Textarea` — the multi-line text sibling, and the closer + analogue — is `string | null` for the same reason. + +### Not changed: `DataTableColumn.key` + +The fourth finding asked for `key: keyof T & string`, so the `#cell-` slot +could type `value` as `T[K]` instead of `unknown`. Declined, on three grounds. + +**It would forbid a column that is not a row property.** An action or computed +column — `{ key: 'actions', label: '' }` rendered through `#cell-actions` — is +a real and supported pattern, and the kit ships no other way to put one +anywhere but the trailing `#row-actions` cell. + +**`SortState.key` is a free string on purpose.** It is emitted to the caller +and, in server mode, straight on to an API. Sort keys naming a joined or +computed column that is not in the row DTO are ordinary. Narrowing the column +key without narrowing that leaves the two disagreeing. + +**And for most row types it would buy nothing at all.** `DataTable` is +`>`. An *interface* only satisfies that +constraint by declaring the index signature — which is what this package's own +`DataTable` story does, and what a consuming app's row types have to do — and +once `[key: string]: unknown` is present, `keyof T & string` **is** `string` and +`T[K]` **is** `unknown`. The proposed narrowing is the identity function on +exactly the row shapes it was proposed for. It would only bite for rows +declared as type aliases, which get an implicit index signature and keep their +exact keys. + +That last point locates the real obstacle: it is the `Record` +constraint, not `key`. Relaxing it (to `T extends object`, with the internal +indexing and the `useSort` signature adjusted to match) is what would make an +exact `value` possible, and it is strictly more permissive, so it would break +nobody. It is also a much larger change than a type narrowing, and it is not in +this release. + +Eleven consumer bindings moved from `{ value }` to `{ row }` in the meantime, +which is better code regardless: `row.name` is exact today, under both row +shapes, with no change to this package. + +### Upgrade notes + +**Install `@codemirror/lang-yaml`.** + +```bash +npm install --save-dev @codemirror/lang-yaml +``` + +It is declared optional in `peerDependenciesMeta`, in step with every other +`@codemirror/*` grammar here, but "optional" describes the manifest and not +what a bundler does. `dist/flows.js` is a single file that dynamic-imports +every grammar by bare specifier, so a consuming build resolves all of them +whether or not the app ever renders an editor — which is already true of +`lang-json`, `lang-markdown`, `commands`, `language`, `state` and `view` today, +and is why every existing consumer already has that set installed. This release +adds one more package to it. An app that upgrades without installing it will +fail to resolve the import at build time, not at runtime. + +Two smaller things can change behaviour, both only for code that was already +outside the declared types: + +- **An unknown `language` no longer highlights as JSON.** If an untyped call + site was passing something the union does not contain and was, by accident, + getting the JSON grammar, it now gets no grammar and a development warning. + A call site that was passing `"yaml"` and getting JSON gets YAML. +- **`CodeEditorProps['modelValue']` includes `null`.** Reading it out into a + `string` needs a fallback. Passing values *in* is strictly freer. + +`DataTable`'s new warning is development-only and fires once per page, but an +app with tables missing `rowKey` will start seeing it immediately. It is +reporting a real defect in those tables — the rows have no distinct keys — and +not a new requirement. + ## v1.18.0 A types-only release, prompted by a consuming app that stood up a `vue-tsc` diff --git a/package-lock.json b/package-lock.json index 05741cd..ef37047 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@codebar-ag/storybook", - "version": "1.16.0", + "version": "1.19.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@codebar-ag/storybook", - "version": "1.16.0", + "version": "1.19.0", "license": "MIT", "dependencies": { "@fontsource/jetbrains-mono": "^5.3.0", @@ -16,6 +16,7 @@ "@codemirror/commands": "^6.10.4", "@codemirror/lang-json": "^6.0.2", "@codemirror/lang-markdown": "^6.5.0", + "@codemirror/lang-yaml": "^6.1.3", "@codemirror/language": "^6.12.4", "@codemirror/state": "^6.7.0", "@codemirror/view": "^6.43.4", @@ -257,6 +258,22 @@ "@lezer/markdown": "^1.0.0" } }, + "node_modules/@codemirror/lang-yaml": { + "version": "6.1.3", + "resolved": "https://registry.npmjs.org/@codemirror/lang-yaml/-/lang-yaml-6.1.3.tgz", + "integrity": "sha512-AZ8DJBuXGVHybpBQhmZtgew5//4hv3tdkXnr3vDmOUMJRuB6vn/uuwtmTOTlqEaQFg3hQSVeA90NmvIQyUV6FQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@codemirror/autocomplete": "^6.0.0", + "@codemirror/language": "^6.0.0", + "@codemirror/state": "^6.0.0", + "@lezer/common": "^1.2.0", + "@lezer/highlight": "^1.2.0", + "@lezer/lr": "^1.0.0", + "@lezer/yaml": "^1.0.0" + } + }, "node_modules/@codemirror/language": { "version": "6.12.4", "resolved": "https://registry.npmjs.org/@codemirror/language/-/language-6.12.4.tgz", @@ -1101,6 +1118,18 @@ "@lezer/highlight": "^1.0.0" } }, + "node_modules/@lezer/yaml": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/@lezer/yaml/-/yaml-1.0.4.tgz", + "integrity": "sha512-2lrrHqxalACEbxIbsjhqGpSW8kWpUKuY6RHgnSAFZa6qK62wvnPxA8hGOwOoDbwHcOFs5M4o27mjGu+P7TvBmw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@lezer/common": "^1.2.0", + "@lezer/highlight": "^1.0.0", + "@lezer/lr": "^1.4.0" + } + }, "node_modules/@marijn/find-cluster-break": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/@marijn/find-cluster-break/-/find-cluster-break-1.0.3.tgz", diff --git a/package.json b/package.json index 8d6675a..d818b05 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@codebar-ag/storybook", - "version": "1.18.0", + "version": "1.19.0", "description": "codebar-ag DocuHub — shared Vue 3 + Tailwind v4 design-system atoms and tokens, documented in Storybook.", "license": "MIT", "author": "codebar Solutions AG", @@ -42,15 +42,16 @@ "tailwind-merge": "^3.0.0" }, "peerDependencies": { - "tailwindcss": "^4.0.0", - "vue": "^3.5.0", - "apexcharts": "^4.5.0 || ^5.0.0 || ^6.0.0", "@codemirror/commands": "^6.10.0", "@codemirror/lang-json": "^6.0.0", "@codemirror/lang-markdown": "^6.5.0", + "@codemirror/lang-yaml": "^6.1.0", "@codemirror/language": "^6.12.0", "@codemirror/state": "^6.7.0", - "@codemirror/view": "^6.43.0" + "@codemirror/view": "^6.43.0", + "apexcharts": "^4.5.0 || ^5.0.0 || ^6.0.0", + "tailwindcss": "^4.0.0", + "vue": "^3.5.0" }, "peerDependenciesMeta": { "apexcharts": { @@ -68,6 +69,9 @@ "@codemirror/lang-markdown": { "optional": true }, + "@codemirror/lang-yaml": { + "optional": true + }, "@codemirror/commands": { "optional": true }, @@ -79,6 +83,7 @@ "@codemirror/commands": "^6.10.4", "@codemirror/lang-json": "^6.0.2", "@codemirror/lang-markdown": "^6.5.0", + "@codemirror/lang-yaml": "^6.1.3", "@codemirror/language": "^6.12.4", "@codemirror/state": "^6.7.0", "@codemirror/view": "^6.43.4", diff --git a/src/components/organisms/CodeEditor.stories.ts b/src/components/organisms/CodeEditor.stories.ts index 29f1ded..c1b8969 100644 --- a/src/components/organisms/CodeEditor.stories.ts +++ b/src/components/organisms/CodeEditor.stories.ts @@ -30,6 +30,42 @@ async function expectHighlighted(canvasElement: HTMLElement): Promise { }); } +/** + * The exact inverse, for the unknown-language fallback: the document renders, + * and nothing in it is painted. + * + * Measured in colour rather than by counting ``s on purpose — CodeMirror + * emits spans of its own for line decorations, so a zero-span assertion would + * be testing something else. Colour is what a grammar buys, so colour is what + * its absence has to be read from. + */ +async function expectNotHighlighted(canvasElement: HTMLElement): Promise { + // Wait for the document first: an editor that has not mounted yet has no + // coloured tokens either, and would pass this vacuously. + await waitFor(async () => { + await expect(canvasElement.querySelector('.cm-line')?.textContent ?? '').not.toBe(''); + }); + + const content = canvasElement.querySelector('.cm-content') as Element; + const base = getComputedStyle(content).color; + const tokens = [...canvasElement.querySelectorAll('.cm-line span')]; + + await expect(tokens.every((token) => getComputedStyle(token).color === base)).toBe(true); +} + +const sampleYaml = `# Compiled flow definition +name: invoice-intake +version: 3 +steps: + - id: ocr + uses: mistral-document-ai + with: + max_page_count: 50 + - id: extract + uses: llm + prompt: "Extract vendor, invoice number and total." +`; + const meta: Meta = { title: 'Organisms/CodeEditor', component: CodeEditor, @@ -60,6 +96,60 @@ export const Markdown: Story = { play: ({ canvasElement }) => expectHighlighted(canvasElement), }; +/** + * YAML is the mode this editor spent its whole life without. Anything that was + * not `markdown` loaded the JSON grammar, so a consuming app's compiled + * flow-definition page highlighted YAML as JSON from the day it was written. + * + * The assertion is the comment line specifically, and `expectHighlighted` would + * not have done. Run the JSON grammar over the document below and it still + * paints nine tokens — every `:` as punctuation, `3` and `50` as numbers — + * which is both why "some token is coloured" passes on the broken version and + * why nobody noticed: the wrong grammar produced plausible colour rather than + * visible breakage. `#` opens a comment in YAML and nothing at all in JSON, so + * a painted first line is reachable only through `@codemirror/lang-yaml`. + */ +async function expectYamlGrammar(canvasElement: HTMLElement): Promise { + await waitFor(async () => { + const content = canvasElement.querySelector('.cm-content'); + await expect(content).not.toBeNull(); + + const commentLine = [...canvasElement.querySelectorAll('.cm-line')].find((line) => + line.textContent?.startsWith('#'), + ); + await expect(commentLine).not.toBeUndefined(); + + const base = getComputedStyle(content as Element).color; + const painted = [...(commentLine as Element).querySelectorAll('span')].some( + (token) => getComputedStyle(token).color !== base, + ); + await expect(painted).toBe(true); + }); +} + +export const Yaml: Story = { + args: { modelValue: sampleYaml, language: 'yaml' }, + play: ({ canvasElement }) => expectYamlGrammar(canvasElement), +}; + +/** + * A language the editor does not know now renders unhighlighted and warns in + * development, instead of silently reaching for JSON. + * + * The value is passed from a template string, which is how it reaches a + * component in practice: `language` is a typed union, so the only callers who + * can get here are untyped ones — and they are exactly the ones with no + * compiler to tell them. + */ +export const UnknownLanguage: Story = { + render: () => ({ + components: { CodeEditor }, + setup: () => ({ sampleYaml }), + template: '
', + }), + play: ({ canvasElement }) => expectNotHighlighted(canvasElement), +}; + export const ReadOnlyEmpty: Story = { render: () => ({ components: { CodeEditor }, diff --git a/src/components/organisms/CodeEditor.vue b/src/components/organisms/CodeEditor.vue index ee8ae1f..d02310c 100644 --- a/src/components/organisms/CodeEditor.vue +++ b/src/components/organisms/CodeEditor.vue @@ -1,7 +1,9 @@