Skip to content

fix(mix): order variant merges by declared state dependencies - #1016

Merged
leoafarias merged 1 commit into
mainfrom
fix/variant-priority
Aug 10, 2026
Merged

fix(mix): order variant merges by declared state dependencies#1016
leoafarias merged 1 commit into
mainfrom
fix/variant-priority

Conversation

@leoafarias

@leoafarias leoafarias commented Aug 10, 2026

Copy link
Copy Markdown
Member

Related issue

Related to #967 — this covers that issue's Variant priority section, which prescribes exactly this shape:

Replace sorting with a stable linear partition:
active non-widget-state variants, in stored order then active widget-state variants, in stored order

The rest of #967 (inherited-style reactivity, controller allocation, scrolledUnder) is untouched, so the issue stays open.

No issue covers the focus-visible half — FocusVisibleVariant shipped in 2.2.0-beta.3, after #967 was filed.

Description

Style.mergeActiveVariants decided variant priority by class (variant is WidgetStateVariant) instead of by the declared semantics (ContextVariant.widgetStateDependencies).

FocusVisibleVariant declares {WidgetState.focused} but is not a WidgetStateVariant, so it always landed in the low-priority group. Any widget-state variant — .onHovered(...), a selected variant, and so on — silently overrode .onFocusVisible(...) on every property the two shared, regardless of declaration order. Downstream this shows up as a focus ring being replaced by a selected border. The only workaround was to duplicate the .onFocusVisible branch inside each competing variant, which is easy to forget and re-breaks silently.

Priority now keys off the same declaration that already drives state-tracking discovery, so the two can no longer drift apart. That drift is what produced the bug: one concept was being answered by two independent classifications.

Separately, the two-value List.sort was not stable. Dart falls back to an unstable quicksort at 32 elements, so declaration order — which the priority contract depends on — was not preserved in large styles.

Changes

  • mergeActiveVariants groups active variants by widgetStateDependencies.isNotEmpty rather than is WidgetStateVariant.
  • Replaced the List.sort with a linear partition into two groups, preserving declaration order within each by construction.
  • Rewrote the method's dartdoc. The old comment also documented a StyleVariation priority tier that never existed in the code.
  • Added 5 regression tests; 4 of them fail on main.
  • CHANGELOG entry under ## Unreleased.

The widgetStateDependencies getter deliberately stays on ContextVariant rather than moving up to Variant. ContextVariant is the only kind Style.widgetStates walks, so a dependency declared on a NamedVariant or ContextVariantBuilder would raise its priority but never get tracking installed — an incoherent state that hoisting would make expressible. The reasoning is recorded in a comment at the branch.

What the fix changes in practice

Each row below is a case where the old ordering was the defect, not a contract being broken. No API changes; nothing needs migrating.

Case Before After
.variant(selected, red) then .onFocusVisible(blue), both active red blue
.onEnabled(blue).onDark(black), enabled + dark black blue
40 equal-priority variants declared 1→40 variant 26 won variant 40 wins

The middle row deserves a look before merging. onEnabled(...) is not(disabled), so it forwards a widget-state dependency and moves into the high-priority group along with everything else that reads state. It reads like base styling but no longer behaves like it. Swapping the declaration order, or nesting the ambient branch inside onEnabled(...), restores the old result. It is pinned by a test and called out in its own changelog bullet.

The alternative — excluding NotVariant from the promotion — was considered and rejected: it needs a variant is! NotVariant check, reintroducing the priority-by-class coupling this change removes, and it contradicts the downstream fingerprint run that validated the current behavior across 603 recipes.

Custom ContextVariant subclasses that read state inside their closure without overriding the getter keep their existing low-priority placement, which the getter's documentation already calls for.

Review Checklist

  • Testing: 5 tests added. Each of the 4 discriminating ones was verified to fail against an unpatched style.dart and pass with it. The fifth pins declaration order within a group and passes either way by design.
  • Breaking Changes: No API change and no migration — existing code compiles unchanged. Resolved styling differs only in the cases where the old ordering was the defect, so this is filed under ### Fixes in the CHANGELOG. The onEnabled(...) row in the table is the one most likely to surprise, and is called out there in its own bullet.
  • Documentation Updates: CHANGELOG entry and the mergeActiveVariants dartdoc.
  • Website Updates: Nothing in btwld/mix-docs currently documents variant merge priority, so there is nothing to update there. Worth adding separately — the priority rule is not written down anywhere user-facing.

Additional Information

Validation

  • cd packages/mix && flutter test — 2891 passed
  • dart analyze packages/mix — no issues
  • cd packages/mix && dcm analyze . --fatal-style --fatal-warnings — no issues
  • dart format — clean
  • melos run test:dart — passed (mix_lint, mix_generator)

melos run analyze and melos run test:flutter do not pass in full, for reasons that predate this branch. Both were re-run against a stashed tree at main and fail identically:

  • schema:inventory fails on type errors in mix_protocol/tool/inventory_check.dart, and dart analyze fails in 8 packages.
  • mix_winds / mix_winds_example resolve mix from .pub-cache/hosted/pub.dev/mix-2.1.0 instead of the workspace, and mix_winds cannot resolve dependencies at all locally.

Root cause, in one line: widgetStateDependencies was introduced in 2.2.0-beta.3 as the declaration of "this variant reads widget state," but the priority sort was never migrated onto it and kept its original class check.

@github-actions github-actions Bot added the mix label Aug 10, 2026
mergeActiveVariants grouped active variants by class (is WidgetStateVariant)
instead of by the declared semantics (widgetStateDependencies). FocusVisibleVariant
declares {focused} but is not a WidgetStateVariant, so it always merged in the
low-priority group and any widget-state variant silently overrode it on shared
properties regardless of declaration order.

Group by the declaration instead, so priority and state-tracking discovery read
the same source of truth. FocusVisibleVariant, and NotVariant forwarding a
state-driven inner variant, move to the high-priority group and compete by
declaration order.

Also replace the sort with a linear partition. List.sort falls back to an
unstable quicksort at 32 elements, and with 40 equal-priority variants declared
in order it resolved variant 26 as the winner rather than variant 40.

Refs #967
@leoafarias
leoafarias force-pushed the fix/variant-priority branch from d7b0eac to 82530d4 Compare August 10, 2026 17:23
@leoafarias
leoafarias merged commit d49bc13 into main Aug 10, 2026
5 checks passed
@leoafarias
leoafarias deleted the fix/variant-priority branch August 10, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant