ReviewOS

stacks/stx

Button freezes disabled and loading at server render, so it cannot express "disabled while my async action runs" - the most common button state

#1997
Closed glennmichael123 opened this 9 days ago · {{ comments.length }} {{ comments.length === 1 ? 'comment' : 'comments' }}
9 days ago

packages/components — verified on 0.2.321.

Button evaluates disabled and loading once, on the server, and never again:

<script server>
export const disabled = $props.disabled || false
export const loading  = $props.loading  || false
export const isDisabled = disabled || loading
</script>

<script client>
const disabledNow = {{ isDisabled }}    <!-- a snapshot, frozen at render -->
function onClick(event) {
  if (disabledNow) { event.preventDefault(); return }
  emit('click', event)
}
</script>

<button … @if(isDisabled)disabled@endif>

Both the disabled attribute and the client guard come from a value computed before the page was sent. So this does not work:

<Button :disabled="rotating()" @click="rotateKey">Rotate</Button>

rotating() is a signal. On the server it is not yet meaningful, isDisabled resolves false, and the button ships enabled and stays enabled for the life of the page — through the entire async action it was supposed to guard. The same applies to :loading, so the built-in spinner can never actually be triggered by the thing it exists for.

Why this is the blocker rather than a nice-to-have

"Disable me while my own async handler is in flight" is the single most common piece of button state in an application. Double-submit protection, a rotate-key confirmation, a destructive delete, a form submit — all of them.

We counted across six stx apps. Of the buttons that are otherwise a clean drop-in for <Button> (utility classes only, no competing CSS):

rely on a reactive :disabled or :text    22
fully static, safe to migrate            28

So 44% of our otherwise-migratable buttons cannot adopt the component, not for styling reasons but because the component cannot express the state they are in. For those we keep a plain <button :disabled="signal()">, which works correctly today.

The same limitation affects the label: our buttons commonly do :text="rotateLabel()" to swap "Rotate" → "Rotating…". Against <Button> the content is a <slot />, so a :text binding on the component does not reach the inner element either.

The library already has the mechanism

Dialog solves exactly this for its own open prop:

// useReactiveProp wires `:open="parentSignal()"` from the parent into our
// local `isOpen` signal — when the parent's signal changes, bindAttr
// updates the root's `open` attribute, our MutationObserver picks it up,
// and isOpen flips. See stacksjs/stx#1704.
const isOpen = useReactiveProp('open', {{ open }})

so the fix looks like giving Button the same treatment:

const isDisabled = useReactiveProp('disabled', {{ disabled }})
const isLoading  = useReactiveProp('loading',  {{ loading }})

and driving the disabled attribute, aria-busy and the spinner from those signals rather than from the server snapshot. Switch already does the equivalent with useModel('checked', …), so there are two precedents in the package.

I would guess the same freeze affects any component with a server-evaluated state prop that a caller would naturally bind to a signal — Progress's value, Skeleton's animate, Notification's show, Tooltip's show. I have only verified Button.

Happy to send the PR for Button if useReactiveProp is the direction you want — it looks like a small change given Dialog already establishes the pattern.

Sign in to comment on this issue.