Skip to content

feat(mix_generator)!: generate named-constructors-only MixWidget variants - #999

Closed
leoafarias wants to merge 1 commit into
mainfrom
fix/998
Closed

feat(mix_generator)!: generate named-constructors-only MixWidget variants#999
leoafarias wants to merge 1 commit into
mainfrom
fix/998

Conversation

@leoafarias

@leoafarias leoafarias commented Jul 24, 2026

Copy link
Copy Markdown
Member

Rewritten — reduced to the actual change

This PR previously implemented a Mix NamedVariant domain (variants: annotation, EnumVariant, variantsFromEnum, applyVariant) at +1239/−73 across mix, mix_annotations, and mix_generator. That was the wrong shape, and I've replaced it with the minimal change: +147/−64 in mix_generator only.

Previous design preserved at 9181d3d's parent — commit 161214a6 on the force-push record above.

Why it shrank

The published generator already emitted the named constructors. For Remix's FortalButton on mix_generator 2.2.0-beta.2:

const FortalButton({...});           // the only thing that needed to go
const FortalButton.solid({...});     // already worked
const FortalButton.classic({...});   // already worked
// ...all six
final FortalButtonVariant variant;   // needed to be private
style: fortalButtonStyler(variant: this.variant, size: this.size)  // already correct

The variant already reached the recipe through its own parameter. The gap to the goal was subtractive — drop the unnamed constructor, make the field private. Framing it as a NamedVariant domain instead pulled in a mix core release, required rewriting every consuming recipe into variantsFromEnum + per-variant helpers, and introduced a per-frame allocation regression (6 stylers built per rebuild for a 6-variant widget, vs 1). None of that was needed.

What this now does

When a function-backed recipe declares a named, non-nullable enum parameter called variant, the generated widget becomes named-constructors-only:

class FortalButton extends StatelessWidget {
  const FortalButton.solid({super.key, this.size = ..., required this.label})
      : _variant = FortalButtonVariant.solid;
  const FortalButton.soft({...}) : _variant = FortalButtonVariant.soft;

  final FortalButtonVariant _variant;   // private
  final FortalButtonSize size;          // public
  final String label;

  @override
  Widget build(BuildContext context) => RemixButton(
        style: fortalButtonStyle(variant: this._variant, size: this.size),
        label: this.label,
      );
}
  • no unnamed constructor, no public variant parameter, no public variant field
  • the value lives in a private _variant field each named constructor pins
  • build() forwards it back into the recipe's own variant parameter

Recipes are untouched. They keep their existing switch (variant) bodies and keep doing the selecting, so there is no EnumVariant, no variantsFromEnum, no applyVariant — and no change to mix or mix_annotations. Only mix_generator needs a release.

Selection semantics are unchanged: variant must still be selected by factoryParameters for the constructors to be generated; it is then curated back off the public surface. Nullable, positional, and non-enum variant parameters keep the ordinary single-constructor behavior, as does an enum value that collides with a generated widget type parameter.

Dynamic selection remains available by calling the recipe directly:

RemixButton(style: fortalButtonStyle(variant: selected, size: size), label: '...')

Breaking change

Generated widgets backed by an enum variant parameter lose their unnamed constructor and public variant field. This breaks the 2.2.0-beta line, which is acceptable on a beta — and Remix, effectively the only consumer, is migrating to named constructors regardless.

Verification

  • mix_generator: 348 tests passing, dart analyze lib/ test/ clean, dart format clean
  • No changes to mix or mix_annotations, so their test suites are unaffected
  • Nine tests that asserted the old shape were updated; each kept its original intent (doc/deprecation carry-over, generic type forwarding, parameter curation) and gained an explicit assertion that the unnamed constructor is absent

Retractions

Two earlier comments of mine on this PR asked for lazy variant registration in variantsFromEnum to avoid a per-rebuild allocation regression. Both are void — that regression only existed in the design being removed. No mix core change is required.

Closes #998, whose proposal (convention-based EnumVariant discovery) is superseded by this simpler mechanism.

@leoafarias

Copy link
Copy Markdown
Member Author

Validated this against the real Remix recipes it's meant to unblock — 25 @MixWidget functions, 17 of them variant-bearing. The generated shape is exactly right: named-only constructors, private _variant, applyVariant with a loud StateError, and key-based matching (thanks for 161214a6 — the == asymmetry would have bitten us). The variants: + variant-parameter conflict error is also exactly what we need; our migration removes variant from all 17 signatures, so we'd have hit it.

Two things before we can migrate.

1. variantsFromEnum is eager, and the recipe runs on every rebuild

variant_style_mixin.dart:75-77 materialises every value up front:

return variants([
  for (final value in values) VariantStyle<S>(value, styleBuilder(value)),
]);

The generated build() calls fortalButtonStyle(...).applyVariant(_variant) per frame. So FortalButton (6 variants) would construct 6 full ButtonStylers per rebuild where today's switch (variant) recipe constructs 1. Same for FortalIconButton; 2–4× for the rest of the 17. That's a per-frame allocation regression we'd rather not ship into a design system's most-used widgets.

Could VariantStyle take an optional thunk and materialise on first value access? From the outside this looks contained:

  • VariantStyle.value is already a getter (core/style.dart:269), so no caller changes.
  • Nothing on the hot path reads .value for inactive variants — mergeActiveVariants filters on variantAttr.variant (core/style.dart:92-100), widgetStates reads only .variant (core/style.dart:68-73), and applyVariant compares registered.key before touching .value (variant_style_mixin.dart:99).

The hazard we can see is props => [variant, _style] (core/style.dart:301) — equality would force materialisation of every variant and defeat the whole thing. Is there a way to key VariantStyle equality that avoids that, or is memoising the materialised style acceptable given Mix's immutability rules? We're happy to take whatever shape you prefer, and happy to send a PR if you'd rather review than write it. We just don't want the named-constructor win to cost per-frame allocations.

2. We do not need the Styler suffix handled in the generator

The name-inference rule requiring ...Style is fine. We're renaming Remix's fortalButtonStylerfortalButtonStyle on our side, which lets us drop name: from all 25 annotations. Flagging only so a second suffix isn't added on your side — fooStyle and fooStyler both deriving Foo would be an ambiguity for no gain.

On variants: being explicit

#998 originally proposed inferring the enum by a <WidgetName>Variant naming convention. Having now worked through it: explicit is better and we'd like to keep what you built. It's one line, it's compiler-checked, it works when the enum lives elsewhere, and a convention would silently downgrade to an unnamed constructor on a typo. Please consider that part of #998 withdrawn.

What we verified locally at 161214a6

With lazy registration resolved, we'll bump Remix to mix 2.2.0-beta.2 / mix_annotations 2.2.0-beta.2 / mix_generator 2.2.0-beta.3 and migrate all 25 wrappers. Our end-state annotation, for reference — 24 of 25 need nothing but target: and variants::

enum FortalButtonVariant with EnumVariant { classic, solid, soft, surface, outline, ghost }

@MixWidget(target: RemixButton.new, variants: FortalButtonVariant)
ButtonStyler fortalButtonStyle({
  FortalButtonSize size = .size2,
  bool highContrast = false,
}) {
  return ButtonStyler().variantsFromEnum(
    FortalButtonVariant.values,
    (variant) => _fortalButtonVariantStyle(variant, size, highContrast),
  );
}

@leoafarias

Copy link
Copy Markdown
Member Author

Refining the props hazard I raised above, having traced it properly — it's narrower than I made it sound.

Style equality is only forced on one path: StyleProvider.updateShouldNotify (core/providers/style_provider.dart:20) does style != oldWidget.style, and StyleProvider is constructed only when inheritable is true (core/style_builder.dart:158-160). Everything on the normal build path — mergeActiveVariants, widgetStates, applyVariant — reads .variant and never .value for inactive variants.

So a memoising thunk would deliver the win on the common path, and degrade to today's behaviour (materialise all, once) only for inheritable: true styles. That seems like an acceptable trade rather than a blocker, and it makes the change smaller than my first comment implied.

Sorry for the two-parter — wanted the correction on the record before anyone acts on the broader version.

@leoafarias

Copy link
Copy Markdown
Member Author

Closing this — it solves the wrong problem, and I want the reasoning on the record.

First, retracting my own two comments above. I asked for lazy variant registration in variantsFromEnum because the generated build() would construct every variant's style per rebuild. That concern was real within this design — but the design isn't needed, so neither is the fix. Please disregard both comments; no mix core change is required.

What the published generator already does

mix_generator 2.2.0-beta.2 already emits, for a recipe with a variant enum parameter:

const FortalButton({...});           // <- the only thing we want removed
const FortalButton.classic({...});   // <- already works
const FortalButton.solid({...});     // <- already works
// ...all six named constructors
final FortalButtonVariant variant;   // <- want this private
style: fortalButtonStyler(variant: this.variant, size: this.size)  // <- already correct

Named constructors ship today, and the variant already reaches the recipe through its own parameter. The gap between that and the goal is subtractive: drop the unnamed constructor, rename the field to _variant. Roughly twenty lines.

Why this PR goes the wrong way

It reframes the enum as a Mix NamedVariant domain, which pulls in EnumVariant, variantsFromEnum, applyVariant, and a mix core release — and then forbids the variant parameter, forcing every consuming recipe to be rewritten into variantsFromEnum + a private per-variant function. For the downstream consumer that was 17 recipe rewrites plus a per-frame allocation regression, to remove one constructor.

The NamedVariant framing came from #998 and was never the requirement. The requirement was only ever "named constructors, no public variant".

What replaces it

A mix_generator-only change making named-constructors-only the default whenever a recipe declares a variant enum parameter:

  • skip the unnamed constructor when a variant domain exists
  • curate variant out of the widget's public parameters
  • declare final <Enum> _variant;
  • forward it as variant: this._variant

No mix change, no mix_annotations change, no variants: annotation field, no applyVariant/variantsFromEnum. Consuming recipes keep their existing switch (variant) bodies untouched, and only mix_generator needs a release.

This is a breaking change to the 2.2.0-beta line for anyone relying on the unnamed constructor, which is acceptable on a beta.

Branch fix/998 is left intact for reference. Closing #998 alongside this.

@leoafarias leoafarias closed this Jul 25, 2026
@leoafarias leoafarias reopened this Jul 25, 2026
…ants

An enum `variant` factory parameter already derived one named constructor per
enum value, but the widget also kept an unnamed constructor and exposed
`variant` as a public field. That gave every variant widget two ways to say the
same thing, and modelled a recipe discriminator as widget state.

The variant is now curated off the widget's public surface: no unnamed
constructor, no public `variant` parameter, no public `variant` field. Each
named constructor pins its enum value into a private `_variant` field, and
`build()` forwards that field back into the recipe's own `variant` parameter.

Recipes are untouched — they keep selecting the style themselves, so no
`EnumVariant`, `variantsFromEnum`, or `applyVariant` is involved and the `mix`
and `mix_annotations` packages need no change. Dynamic selection stays
available by calling the recipe directly and passing the result through the
target's `style` parameter.

Selection semantics are unchanged: `variant` must still be selected by
`factoryParameters` for the constructors to be generated.

BREAKING CHANGE: generated widgets backed by an enum `variant` factory
parameter no longer expose an unnamed constructor or a public `variant` field.
@leoafarias leoafarias changed the title feat(mix_generator): generate named-only MixWidget constructors from enum variants feat(mix_generator)!: generate named-constructors-only MixWidget variants Jul 25, 2026
@leoafarias

Copy link
Copy Markdown
Member Author

Force-pushed a full rewrite: 161214a69181d3d2. Scope dropped from +1239/−73 across three packages to +147/−64 in mix_generator only. The PR description above has been replaced; see it for the reasoning. My two earlier comments asking for lazy variant registration are retracted — no mix core change is needed.

@leoafarias

Copy link
Copy Markdown
Member Author

Final decision — #999, #998, and what's changing next

Thanks for the rewrite and the blast-radius report. Both were genuinely useful —
the report in particular is what settled the decision, just not in the direction
you expected.

1. Decision: named-only is declined. #999 is closed unmerged.

Your 9181d3d2a is better work than the original #999packages/mix
untouched, no variantsFromEnum/applyVariant, recipes keeping switch (variant), one style per rebuild, ~147 lines instead of ~1,141. The mechanism
reasoning in it is right, and it's the shape we'd have asked for.

But "Confirmed named-only" was never confirmed. The decision is the opposite:
the unnamed constructor and the public variant parameter stay.

The deciding evidence is from your own report:

16 demo knobs → RemixButton(style: fortalButtonStyle(variant: knob, ...), ...)

That's the case against named-only, measured. When a variant comes from state,
config, or a UI control, no named constructor can be selected at compile time —
so 16 call sites abandon the generated FortalButton wrapper and drop to the raw
target with a hand-built style. Preventing exactly that is what the wrapper is
for.

Supporting reasons:

  • feat(mix_generator): generate variant widget constructors #968 preserved the unnamed constructor as an explicit non-breaking guarantee,
    and that reason still holds.
  • Both spellings cost nothing to keep. Write FortalButton.solid(...) everywhere
    you want it; the unnamed form is there for the cases that need it.
  • Under named-only, every recipe's variant = .solid default becomes silently
    dead code, since each constructor pins a value. The generator doesn't diagnose
    it.

#998 is closed too. Its core ask was the named-only shape; the capability it
was reaching for already ships.

2. What you can do today — no Mix release needed

mix_generator 2.2.0-beta.2 is already published and already generates variant
constructors. From #968 (2524b74ad).

  1. mix_generator: ^2.2.0-beta.2 in dev_dependencies
  2. Leave mix and mix_annotations alone — no upgrade, no republish
  3. Keep variant in all 17 recipes. Removing it turns the feature off — the
    parameter is the trigger.
  4. Recipe bodies unchanged (switch (variant))
  5. Rename fortalButtonStylerfortalButtonStyle as planned, dropping name:
  6. Use FortalButton.solid(...), and keep FortalButton(variant: knob, ...) for
    the 16 dynamic sites

The ~74 call-site conversions, ~20 field-read deletions, 38 example sites, and 26
doc pages in your report are not needed.

Generation requires all five of: recipe is a function; parameter named exactly
variant; named not positional; non-nullable enum; survives factoryParameters
(see §3 — that last condition is being removed). The parameter does not need
to be required, and the enum does not need EnumVariant — a plain enum is
correct, since the value is only a switch key and never enters Mix's variant
system.

Note the versions in your earlier comment — mix 2.2.0-beta.2,
mix_annotations 2.2.0-beta.2, mix_generator 2.2.0-beta.3 — were from the
closed branch and won't be published as described. Currently published: mix 2.2.0-beta.1, mix_annotations 2.2.0-beta.1, mix_generator 2.2.0-beta.2.

3. Coming next: a breaking change in mix_generator 2.2.0-beta.3

Your factoryParameters gotcha is being fixed by removing the feature that
causes it. Plan for this before you finish migrating.

Miss 'variant' in that Avatar list and FortalAvatar.soft() / .solid()
vanish with no error.

What changes

  1. factoryParameters is removed. All recipe function parameters are always
    exposed on the generated widget.
  2. widgetParameters is renamed targetParameters. Once it's the only
    curation knob, "widget parameters" is misleading — recipe parameters also
    become widget parameters. The knob curates the target widget's constructor
    surface, so it should say so.

Why

A recipe function's parameters are your own API — you author both the function
and the annotation. If a parameter shouldn't reach the widget, don't declare it.
Curation earns its place on the surface you don't control: the target widget's
constructor. That was the original split (widgetParameters came in #974 on its
own; factoryParameters arrived later bundled into #997), and this restores it.

Your gotcha disappears by construction: with no curation of recipe parameters,
variant can never be filtered out, so the constructors can never silently
vanish.

What you'll need to change

  • AvatarfactoryParameters: .only({'variant', 'size', 'highContrast'})
    goes away, so fallbackLength becomes a public widget parameter. Three options:
    accept it exposed; move it out of the recipe signature and compute it
    internally; or keep it and document it. Tell us which you prefer — if
    exposing it is genuinely harmful, say so now, because that's the one input
    that could still change this design.
  • Any widgetParameters: usage → rename to targetParameters:
  • The other 24 components use .all() implicitly and need no change

One thing you lose

factoryParameters doubles today as an escape hatch when a recipe parameter and
a target parameter share a name with different types — the generator currently
tells you to exclude it from either side. Afterwards you can only exclude from
the target side. We think that's correct rather than a regression: you control
your recipe's parameter names and can rename to avoid the collision, but you
don't control the target's. Flag it if you have a case where that's not true.

4. Timeline

  1. Migrate now against published 2.2.0-beta.2 — keep variant params, keep
    factoryParameters where you use it today
  2. Answer the Avatar question above
  3. We ship mix_generator 2.2.0-beta.3 with §3's breaking changes
  4. You drop factoryParameters from Avatar and rename any widgetParameters

Steps 1 and 3 are independent — you don't need to wait on us to start.

5. Process note

Please don't self-merge or publish to pub.dev from the requesting side.
Publishing is irreversible. And a force-push that replaces an open PR's entire
design should come with a heads-up rather than a merge request — the
implementation was good, it just answered a question that hadn't been decided.

6. What would still change our minds

  • A concrete case where the unnamed constructor's existence causes a real
    problem, as opposed to a preference for a smaller public surface
  • A recipe where the variant genuinely can't be a parameter
  • Exposing Avatar's fallbackLength being actually harmful rather than untidy
  • A measured frame-time regression from the parameter form — it builds one style
    per rebuild, identical to your current recipes, so this would be surprising

@leoafarias

Copy link
Copy Markdown
Member Author

Closing unmerged per the decision above. The variant constructor capability ships today in the published mix_generator 2.2.0-beta.2; nothing here needs to land.

Design record for all of this is in #1000.

@leoafarias leoafarias closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generate named-only @MixWidget constructors from enum-backed NamedVariants

1 participant