v1.16.1 — three components cached a slot check Vue never invalidates

CHANGELOG.md

@@ -5,6 +5,141 @@ 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.16.1
+
+One bug, found in a consuming app, and the two more the sweep for its shape
+turned up. Nothing gains a prop, a slot, a token or a class; every call site
+that renders correctly today renders byte-identically after this.
+
+### Fixed
+
+- **`Card` rendered no header — and so no `#actions` — when its only slot
+  arrived after mount.** A card with neither `title` nor `description`, whose
+  `#actions` template sits behind a `v-if`, never grew a header when the
+  condition turned true. The consuming symptom was a page on which the Save
+  button did not exist: the form could only be submitted by pressing Enter in
+  its single text input, and implicit submission is the only reason that page
+  was not a hard block. Card's own inner `v-if="$slots.actions"` was correct the
+  whole time and simply unreachable, which is why reading the component does not
+  find this and only trying it does.
+
+  The header was decided by a `computed`:
+
+  ```js
+  const hasHeader = computed(
+      () => props.title !== null || props.description !== null || !!slots.title || !!slots.actions,
+  );
+  ```
+
+  **`useSlots()` does not return a reactive object.** Vue builds it once
+  (`instance.slots = createInternalObject()`) and `updateSlots` **mutates that
+  same object in place** on every parent render — the identity never changes and
+  no proxy is watching, so nothing invalidates a computed over it. With no
+  `title` and no `description` the expression evaluated `false` during mount and
+  cached that for the lifetime of the card. The card *did* re-render — a
+  conditional slot makes its slots non-stable, which forces the child to update
+  — but a cached computed does not care that a render happened.
+
+  The trap has a lid on it, and that is the part worth recording. Vue does track
+  `$slots`: reading it through the public instance proxy calls
+  `track(instance, 'get', '$slots')`, and in development `useSlots()` returns a
+  proxy that does the same. Grep the matching `trigger(instance, 'set',
+  '$slots')` in `@vue/runtime-core` 3.5 and there is **exactly one**, inside
+  `updateSlots`, behind `if (isHmrUpdating)`. A computed over slots is therefore
+  invalidated by hot module replacement and by nothing else: it recomputes while
+  you edit the file — precisely when you would be looking at it — and never once
+  in a built application. Switching the computed from `slots` to `$slots` would
+  have looked like the fix and would have shipped the same bug.
+
+  There is no version of this that is safe to cache, so the header is decided in
+  the render and only in the render:
+
+  ```html
+  <header v-if="title !== null || description !== null || $slots.title || $slots.actions">
+  ```
+
+  A template *is* a render function, so the read happens once per render, after
+  `updateSlots` has finished mutating the object — the only moment at which the
+  answer is knowable. It is also what fourteen other components in this kit
+  already do (`Modal`, `Drawer`, `Toggle`, `AppShell`, `Navbar`, …), so the fix
+  deletes an outlier rather than introducing a mechanism.
+
+  Two alternatives were rejected. **Mirroring the slot names into a `ref` from
+  an `onBeforeUpdate` hook** does work and keeps a computed — but it makes a
+  second copy of state Vue already owns, correct only for as long as the hook
+  stays wired to it, in exchange for caching a four-term boolean. **Rendering
+  the `<header>` unconditionally** and letting its contents collapse is the one
+  option here that would be breaking: every plain card would gain an empty
+  padded strip and a bottom border it does not have today.
+
+- **`MetricGrid` counted its caller's syntax instead of its caller's tiles.**
+  The filler tiles that stop the hairline-gap background showing through as a
+  stray solid block were sized from `slots.default().length` — the length of a
+  vnode array, which is not the number of tiles in either shape a real caller
+  writes. `v-for` collapses its whole run into a single Fragment, so **three
+  metrics read as one** and a nearly full row was padded with three fillers and
+  drawn as a quarter-full one. `v-if` leaves a Comment placeholder behind when
+  false, so **three metrics plus a hidden fourth read as four** and the filler
+  the gap actually needed was never drawn. Both counts are now taken after
+  flattening fragments and dropping comment and whitespace placeholders, and —
+  same rule as `Card` — in the render rather than in a `computed`, so a grid
+  whose tile count changes re-pads instead of keeping its first answer.
+
+  Grids written with literal children, which is every story and every static
+  call site, are unaffected: they counted correctly before and count correctly
+  now.
+
+- **`DataTable`'s `cursor-pointer` affordance answered for the vnode that
+  existed at mount.** `hasRowClick` was a `computed` over
+  `instance.vnode.props.onRowClick`, and `instance.vnode` is *replaced* on every
+  parent render while `instance` is a plain object — the same defect against a
+  different dependency. It is read per render now.
+
+  The honest scope, because the fix is real but smaller than it looks: written
+  as `@row-click` in an SFC the compiler emits a cached wrapper function, so the
+  key is always present and the affordance was never wrong for that spelling. It
+  is reachable by binding the handler as a value
+  (`:onRowClick="editable ? open : undefined"`), and even then only partly,
+  because Vue's `hasPropsChanged` deliberately skips declared emit listeners:
+  binding `row-click` makes the listener live without asking this component to
+  redraw at all. So the affordance now corrects itself at the next render for
+  any other reason — which in a table is constant — where before it never
+  corrected itself.
+
+### Changed
+
+- **`FormActions`, `PageHeading` and `Table` read `$slots` instead of a
+  `useSlots()` binding.** Behaviour is identical — the same property on the same
+  object, read at the same moment — and this is housekeeping with a point: the
+  hazard was never the read, it is having a `slots` identifier sitting in setup
+  scope where the next `computed` can close over it. `MetricGrid` is now the
+  only component that calls `useSlots()`, because it needs to invoke the slot
+  function rather than test for it.
+
+### Notes
+
+- Each of the three is pinned by a story whose play function fails on the
+  previous code and passes on this one: a slot arriving and leaving after mount,
+  a tile count from `v-for` and from `v-if`, a handler bound late. The suite is
+  242 tests, up from 237.
+- The `Card` defect predates the `#title` slot. The computed has read
+  `slots.actions` since the initial build-out; v1.14.0 only added a second slot
+  to an already-broken expression. Its note there — "the header renders whenever
+  a `title`, `description`, `#title` or `#actions` is present" — was true only
+  of slots present at mount, and is true as written from this release.
+- Adjacent, found by the same sweep and deliberately not changed: `DataTable`
+  passes `comparators.value` into `useSort()`, unwrapping a computed at setup
+  and freezing the per-column `sortFn` map, so a table that swaps its `columns`
+  after mount sorts with the old comparators. Same family — a value read once
+  where it should be read on demand — but the fix is a change to `useSort`'s
+  signature, and that belongs in a release that can look at the composable's API
+  rather than in a patch.
+- `useAttrs()` is **not** an instance of this, which is most of why the slots
+  version survived so long. It looks identical, but Vue backs attrs with a real
+  `track`/`trigger` pair that fires on every props update, so the computeds in
+  `useRootAttrs` and `usePasswordManagerAttrs` are correctly reactive. Slots are
+  the exception, not the rule.
+
 ## v1.16.0
 
 The non-colour channel v1.15.0 said would be needed. Additive and non-breaking:

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@codebar-ag/storybook",
-  "version": "1.16.0",
+  "version": "1.16.1",
   "description": "codebar-ag DocuHub — shared Vue 3 + Tailwind v4 design-system atoms and tokens, documented in Storybook.",
   "license": "MIT",
   "author": "codebar Solutions AG",

src/components/molecules/Card.stories.ts

@@ -1,4 +1,6 @@
 import type { Meta, StoryObj } from '@storybook/vue3-vite';
+import { expect, userEvent, waitFor, within } from 'storybook/test';
+import { ref } from 'vue';
 import Card from './Card.vue';
 import Button from '../atoms/Button.vue';
 import StatusBadge from '../atoms/StatusBadge.vue';
@@ -49,6 +51,77 @@ export const TitleSlot: Story = {
     }),
 };
 
+// The header's existence is decided per render, not once at mount. A card with
+// no `title` and no `description` starts with no header at all; the moment a
+// conditional `#actions` template turns on, the header — and the actions it
+// contains — has to appear, and has to go again when it turns off. This is the
+// shape a form card takes: the Save button only exists once the form is
+// complete enough to submit, and the card carrying it has no title.
+export const ActionsAppearingAfterMount: Story = {
+    render: () => ({
+        components: { Card, Button },
+        setup: () => ({ ready: ref(false) }),
+        template: `<Card>
+            <template
+                v-if="ready"
+                #actions
+            ><Button size="sm">Save</Button></template>
+            <Button
+                size="sm"
+                variant="secondary"
+                @click="ready = !ready"
+            >{{ ready ? 'Reset' : 'Complete form' }}</Button>
+        </Card>`,
+    }),
+    play: async ({ canvasElement }) => {
+        const canvas = within(canvasElement);
+
+        await expect(canvas.queryByRole('button', { name: 'Save' })).not.toBeInTheDocument();
+
+        await userEvent.click(canvas.getByRole('button', { name: 'Complete form' }));
+        await waitFor(() => expect(canvas.getByRole('button', { name: 'Save' })).toBeVisible());
+
+        await userEvent.click(canvas.getByRole('button', { name: 'Reset' }));
+        await waitFor(() =>
+            expect(canvas.queryByRole('button', { name: 'Save' })).not.toBeInTheDocument(),
+        );
+    },
+};
+
+// Same rule for `#title`: a card that gains a title after mount gains a header.
+export const TitleAppearingAfterMount: Story = {
+    render: () => ({
+        components: { Card, Button },
+        setup: () => ({ loaded: ref(false) }),
+        template: `<Card>
+            <template
+                v-if="loaded"
+                #title
+            >Nightly sync</template>
+            <Button
+                size="sm"
+                variant="secondary"
+                @click="loaded = !loaded"
+            >{{ loaded ? 'Unload' : 'Load' }}</Button>
+        </Card>`,
+    }),
+    play: async ({ canvasElement }) => {
+        const canvas = within(canvasElement);
+
+        await expect(canvas.queryByRole('heading', { name: 'Nightly sync' })).not.toBeInTheDocument();
+
+        await userEvent.click(canvas.getByRole('button', { name: 'Load' }));
+        await waitFor(() =>
+            expect(canvas.getByRole('heading', { name: 'Nightly sync' })).toBeVisible(),
+        );
+
+        await userEvent.click(canvas.getByRole('button', { name: 'Unload' }));
+        await waitFor(() =>
+            expect(canvas.queryByRole('heading', { name: 'Nightly sync' })).not.toBeInTheDocument(),
+        );
+    },
+};
+
 export const Small: Story = {
     render: () => ({
         components: { Card },

src/components/molecules/Card.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { computed, useSlots } from 'vue';
+import { computed } from 'vue';
 import { cx } from '../../helpers/cx';
 import { useRootAttrs } from '../../composables/useRootAttrs';
 import { pick } from '../../helpers/pick';
@@ -22,7 +22,6 @@ const props = withDefaults(
     { title: null, description: null, padded: true, variant: 'default', hoverable: false, size: 'md' },
 );
 
-const slots = useSlots();
 const { rootAttrs, classAttr } = useRootAttrs();
 
 const variants: Record<string, string> = { default: '', danger: 'border-danger-line/60' };
@@ -56,19 +55,21 @@ const rootClass = computed(() =>
         classAttr.value,
     ),
 );
-
-const hasHeader = computed(
-    () => props.title !== null || props.description !== null || !!slots.title || !!slots.actions,
-);
 </script>
 
 <template>
   <div
     :class="rootClass"
     v-bind="rootAttrs"
   >
+    <!-- Read `$slots` here, in the render, and never through a `computed`. The
+         object `useSlots()` returns is built once and mutated in place, so it
+         is not a reactive dependency: a computed over it caches whatever was
+         true at mount. A card with no `title` and no `description` would then
+         cache "no header" forever, and a `#actions` that arrived later — the
+         normal shape for a form's Save button — could never appear. -->
     <header
-      v-if="hasHeader"
+      v-if="title !== null || description !== null || $slots.title || $slots.actions"
       :class="['flex flex-col gap-3 sm:flex-row sm:items-start sm:justify-between sm:gap-4 border-b border-line', headerPadding[size]]"
     >
       <div class="min-w-0">
@@ -79,7 +80,7 @@ const hasHeader = computed(
              state of the thing the card is about — instead of being exiled to
              `#actions` on the far side of the header. -->
         <h2
-          v-if="title !== null || slots.title"
+          v-if="title !== null || $slots.title"
           :class="['flex flex-wrap items-center gap-x-3 gap-y-1 font-semibold text-ink', titleSize[size]]"
         >
           <slot name="title">
@@ -94,7 +95,7 @@ const hasHeader = computed(
         </p>
       </div>
       <div
-        v-if="slots.actions"
+        v-if="$slots.actions"
         class="flex flex-wrap items-center gap-2 sm:shrink-0"
       >
         <slot name="actions" />
@@ -106,7 +107,7 @@ const hasHeader = computed(
     </div>
 
     <footer
-      v-if="slots.footer"
+      v-if="$slots.footer"
       :class="[footerPadding[size], 'bg-surface-2 border-t border-line text-xs text-muted rounded-b-surface']"
     >
       <slot name="footer" />

src/components/molecules/FormActions.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-import { computed, useSlots } from 'vue';
+import { computed } from 'vue';
 import { pick } from '../../helpers/pick';
 
 // Form-footer action row. `between` puts the secondary action (Cancel) on the
@@ -10,8 +10,6 @@ const props = withDefaults(
     { align: 'between' },
 );
 
-const slots = useSlots();
-
 const alignments: Record<string, string> = {
     between: 'justify-between',
     end: 'justify-end',
@@ -24,7 +22,7 @@ const alignment = computed(() => pick(alignments, props.align, 'between', 'FormA
 <template>
   <div :class="['flex items-center gap-2 pt-2', alignment]">
     <slot
-      v-if="slots.secondary"
+      v-if="$slots.secondary"
       name="secondary"
     />
     <slot />

src/components/molecules/PageHeading.vue

@@ -11,7 +11,7 @@
 // consuming app was otherwise re-pairing them by hand on every nested page.
 // Pass `breadcrumbAs` (e.g. Inertia's `Link`) for SPA navigation, exactly as
 // Breadcrumbs' own `as` prop expects.
-import { computed, useSlots } from 'vue';
+import { computed } from 'vue';
 import { cx } from '../../helpers/cx';
 import { useRootAttrs } from '../../composables/useRootAttrs';
 import Breadcrumbs, { type BreadcrumbItem } from './Breadcrumbs.vue';
@@ -27,7 +27,6 @@ withDefaults(
     { eyebrow: null, breadcrumbs: () => [], breadcrumbAs: 'a' },
 );
 
-const slots = useSlots();
 const { rootAttrs, classAttr } = useRootAttrs();
 
 const classes = computed(() => cx('text-xl font-semibold text-ink', classAttr.value));
@@ -61,14 +60,14 @@ const classes = computed(() => cx('text-xl font-semibold text-ink', classAttr.va
           <slot />
         </h1>
         <p
-          v-if="slots.description"
+          v-if="$slots.description"
           class="mt-1 text-sm text-muted"
         >
           <slot name="description" />
         </p>
       </div>
       <div
-        v-if="slots.actions"
+        v-if="$slots.actions"
         class="flex flex-wrap items-center gap-2 shrink-0"
       >
         <slot name="actions" />