Skip to content
Development

The callback that couldn't say what happened

By Victor Da Luz
svelterusttauridev-loggreenhouse

Two small, unrelated-looking bugs from my last code review of Greenhouse turned out to share a root cause I’d already “fixed” once before, in the same file, a few days earlier. That was the useful part of this one - not the fix itself, but noticing the fix from last time hadn’t actually finished the job.

Bug one: importing a project made it vanish

Greenhouse lets you adopt a folder you already had on disk into the app’s pipeline. You open a “Switch vault” dialog, it scans the folder you’re pointed at, and you assign each unclaimed subfolder to a stage. Click “Import & Continue” and the item should show up on your worklist.

It didn’t, if you were importing into the vault you were already using. The import itself worked fine - database row written, files where they should be - but the dashboard never knew to look again. Tracing it back: the picker component has exactly two ways to tell its caller something happened. One fires when you point it at a brand new folder. The other fires when there’s nothing left to decide. Importing into your current folder is neither of those. It’s a real event with no way to announce itself.

Bug two: I’d already written the lesson for this

Here’s where it got interesting. A few days earlier I’d fixed a different bug in the exact same component, and written myself a note about it afterward: when you extract a component that used to serve one caller into something reused by two, audit every callback for context it was quietly relying on. That earlier fix made the “nothing left to decide” signal fire only when the situation actually called for it, instead of misfiring the moment a dialog opened.

That fix was correct. It also gave me false confidence that the component’s design was now sound, when all I’d actually fixed was when an existing signal fired. I hadn’t asked whether the small set of signals it offered could even describe every real event the component could produce. It couldn’t. “I just switched to a different folder” and “I just imported something into the folder I was already in” are different facts about the world, and the component only had room to say one of them.

The fix this time was to stop treating the signal set as fixed and add a third one - a narrow callback that fires only when an import actually happened, wired directly to a plain data refresh. Not routed through the “vault switched” handler, which resets a pile of other UI state that has nothing to do with this case. I went back and added a section to my own note about the first fix, because the second bug isn’t a new lesson, it’s the first lesson applied one level too shallow: fixing when a callback fires doesn’t tell you whether you have enough callbacks in the first place.

Bug three, which wasn’t a bug

The same review flagged a third thing, and this one I almost fixed before checking it was real. Greenhouse has an in-app audio previewer - click a file, a dialog opens, you can listen without leaving the app. Advancing a project moves its folder on disk, so the theory was: if you have the preview dialog open and then advance the stage, the player keeps pointing at a file path that no longer exists.

Except you can’t actually do that. The preview dialog uses the browser’s native <dialog> element, opened with its real showModal() method, not a hand-rolled overlay. That method makes everything else on the page properly inert - the “Confirm advance” button included. You physically cannot click it while a preview is open. You have to close the preview first, which already clears its state. The bug I was about to write defensive code for couldn’t happen, because the platform had already ruled it out.

I still had a real, adjacent bug to fix, though - the file list sitting behind that dialog stayed mounted the whole time an item’s detail view was open, and it only ever loaded once, at the start. Advancing a stage moved the files on disk and the list never noticed. That one just needed a second fetch alongside the existing refresh, once I’d confirmed it was the actual reachable path rather than the one blocked by the modal.

What I’d take from this

Two things, and they pull in slightly different directions. First: fixing a bug in a shared component is a good moment to ask a second question you don’t automatically ask - not just “does this fire at the right time now,” but “can this component’s whole vocabulary of signals even describe everything it can do.” The second question is more work and it’s tempting to skip it once the visible bug is gone. Second: a plausible-sounding bug report is worth ten minutes of checking whether the interleaving it describes is even reachable before you write a line of defensive code for it. Sometimes the platform already did the fix for you, and the actual work is confirming that, not adding to it.

Related reading