Clear the Lineup — Astro's `popover` Attribute Had No Way to Say False

This is a submission for DEV's Summer Bug Smash: Clear the Lineup.

Project Overview

Astro is the content-first web framework much of the JS ecosystem reaches for — docs sites, blogs, increasingly full apps — built around server-rendering .astro templates to plain HTML with an island-based hydration model. Buried in that renderer is a small, unglamorous function called addAttribute(), whose entire job is deciding what string, if any, gets printed for a given prop value and attribute name. Nobody thinks about it — until it starts emitting invalid HTML on every page using one specific attribute on a custom element.

That's issue #17398: "For Custom HTML Tags Astro Compile Rewrites popover Attribute to Invalid Value."

Bug Fix or Performance Improvement

The problem: here's a piece of web-platform trivia I didn't know until this bug bit me: the Popover API has no valid "false". popover isn't a plain boolean HTML attribute like disabled or checked — it's an enumerated attribute with exactly three legal states: absent, "auto", or "manual". There is no string spelling of "off." Write popover="false" into the DOM and the browser sees an attribute present with an invalid value, which per spec falls back to a valid popover state. So popover="false" doesn't disable a popover — it enables one.

That's exactly what Astro was doing on custom elements. Write <el-popover popover class="..."> in a .astro file, and the compiled output was <el-popover popover="true" class="...">. The issue's reporter had a working StackBlitz repro on Astro 7.0.9 with Tailwind Plus Elements, showing this silently breaking light-dismiss (click-outside-to-close) behavior — no console warning, no type error, nothing pointing at the actual cause. It's the kind of bug you only find by staring at rendered HTML and knowing the spec well enough to notice something's off.

The hunt

I'll be upfront about how this went: I worked through it with Claude, using Claude Code as an agentic pair rather than reading 500+ files solo. The initial pass had already narrowed things to handleBooleanAttribute() in packages/astro/src/runtime/server/render/util.ts and named two suspect PRs. A named suspect isn't a confirmed root cause, though, so the real work was reading the current source myself and checking the story against it line by line.

It held up, and it's a genuinely nice case of two individually-correct PRs colliding:

• PR #13037 taught addAttribute() that popover is special: it's boolean-shaped (true/false in JS) but also accepts the strings "auto"/"manual", so it can't just live in the simple htmlBooleanAttributes regex like hidden does. It routes popover through the shared handleBooleanAttribute() helper instead.
• PR #13909, later and unrelated, taught that same shared helper a different rule: for custom elements (any tag name containing a hyphen), always stringify boolean attributes into key="true"/key="false", fixing real bugs where Lit components and things like autoplay need an actual attribute value rather than presence/absence, matching how custom-element property reflection works.

Neither PR was wrong on its own. They just both modified the same chokepoint function for opposite reasons, and popover is the one attribute that's boolean-shaped in JS but has no valid "off" string in HTML — so it fell straight through the crack. The function was checking "is this a custom element?" before it ever asked "is this specifically popover?" — so popover inherited stringification semantics meant for a completely different problem (Lit prop reflection).

Code

PR: https://github.com/withastro/astro/pull/17422

The fix, in packages/astro/src/runtime/server/render/util.ts:

 function handleBooleanAttribute(
    key: string,
    value: boolean | string,
    shouldEscape: boolean,
    tagName?: string,
 ): string {
+   // The Popover API only accepts "auto", "manual", or the attribute being absent.
+   // There's no valid string value for "off", so it must always be rendered as a
+   // boolean attribute, even on custom elements.
+   if (key === 'popover') {
+       return markHTMLString(value ? ` ${key}` : '');
+   }
    // For custom elements, always render as string attributes
    if (tagName && isCustomElement(tagName)) {
        return markHTMLString(` ${key}="${toAttributeString(value, shouldEscape)}"`);

Six lines. A guard that checks popover first and always returns it as a bare-or-omitted boolean attribute, regardless of tag name, before the custom-element stringify branch ever runs. download and hidden — the other two attributes that flow through this same helper — are deliberately left untouched, since their custom-element stringification is correct and unrelated to this bug.

I added a matching regression test to the existing suite, packages/astro/test/units/app/astro-attrs.test.ts:

     <custom-el id="popover-custom-el-true"${addAttribute(true, 'popover', true, 'custom-el')} />
     <custom-el id="popover-custom-el-false"${addAttribute(false, 'popover', true, 'custom-el')} />
     <custom-el id="popover-custom-el-auto"${addAttribute('auto', 'popover', true, 'custom-el')} />

with expectations:

            'popover-custom-el-true': { attribute: 'popover', value: '' },
            'popover-custom-el-false': { attribute: 'popover', value: undefined },
            'popover-custom-el-auto': { attribute: 'popover', value: 'auto' },

plus a changeset documenting the patch for the release notes. Three files, 18 lines, all additions.

My Improvements

Confirming the bug before touching code. Before writing a fix, I reproduced the failure using the test file's own existing convention — calling addAttribute() directly rather than compiling a full .astro file, since every other popover case in that file already does it that way. Adding the custom-element case and running the suite gave a clean, unambiguous failure:

AssertionError [ERR_ASSERTION]: Expected popover to be  for #popover-custom-el-true
'true' !== ''
actual: 'true', expected: '', operator: 'strictEqual'

That's the bug, byte for byte: the renderer emitting popover="true" where it should emit bare popover.

Writing a guard, not a special case pile-up. The tempting quick fix is an if (tagName?.includes('-')) check tucked into addAttribute() itself. I didn't want popover logic living in two places, so the guard went directly inside handleBooleanAttribute(), ahead of the custom-element branch. Every caller of that function now gets the correct behavior for free, and the diff reads as "popover is exempt from custom-element stringification" rather than a scattered patch.

After the fix, the same test command passed all three new cases — popover-custom-el-true → bare popover, popover-custom-el-false → attribute omitted, popover-custom-el-auto → popover="auto" preserved — and every plain-element popover case kept passing unchanged, since the new branch is byte-identical output to the old fallback on that path. I widened the net past that one file: the class-list/style, directives, html-primitives, slots, hydration, and URL-attribute-XSS suites (124/124), the custom-element escaping suite (18/18), and the JSX custom-elements rendering suite (5/5). 150 passing tests total, directly touching the changed function and its neighbors, zero failures.

I'll be equally honest about what I didn't run: the full 235-file unit suite. Astro is a 517-package monorepo, and a full pnpm install was blocked by an unrelated npm registry trust-downgrade flag on a Netlify integration's dependency — nothing to do with this bug. Scoping the install with pnpm install --filter ...astro got me a working build, but left packages like @astrojs/markdown-remark unbuilt, and since the test runner concatenates all matched files into one process, a single missing import aborts the whole run before anything executes. So I ran the focused, targeted slice instead of claiming a full green suite I didn't actually get.

The toolchain had its own side quest: the repo's pnpm-lock.yaml is a multi-document YAML file — a self-pin block for pnpm@11.10.0 concatenated ahead of the real lockfile — that only pnpm 11.x parses correctly. My globally-installed pnpm 10.x quietly treated it as malformed and fell back to full re-resolution from the registry, which is what surfaced the unrelated trust-downgrade error in the first place. A "broken lockfile" warning was masking a different supply-chain gate one layer down. I also learned that setting NODE_OPTIONS mid-process to add --experimental-strip-types does nothing: Node reads that variable once at boot, and Astro's own test runner needs it passed to the initial node invocation directly, not toggled at runtime.

The broader lesson I'm carrying past this specific bug: not every HTML attribute that looks boolean is boolean. disabled, hidden, and checked are true booleans — present or absent, nothing in between. popover looks like one in JS (true/false), but it's actually an enumerated attribute with a closed set of legal string values and no spelling for "off." Any framework's attribute-serialization layer that treats "this prop is boolean-typed" as equivalent to "render it as a bare HTML boolean attribute" is exactly where enumerated attributes like popover — or tristate ones like aria-checked — will quietly fall through the crack. Two correct PRs, one shared chokepoint function, and the one attribute that doesn't fit either PR's mental model cleanly: that's the whole bug.