[PHP 8.3] Filament\Support\Enums\Alignment::tryFrom(): Passing null to param 1($value) of type string|int is deprecated

[drbyte]

Package

filament/filament

Package Version

v3.0.96

Laravel Version

v10.32.1

Livewire Version

v3.1.0

PHP Version

PHP 8.3.0-RC5

Problem description

Discovered when using a deeply-nested Infolist modal in a Resource's RelationManager's ViewAction where the TextEntry fields are triggering the following warning. Discovered via the 'Messages' tab of Laravel DebugBar.

Some debugging revealed that it was being triggered in:

(which I think is called from ... )

Expected behavior

Obviously: expecting no deprecation warnings when no alignment value is specified when the component is declared in the resource or relationmanager ... or anywhere else.

Steps to reproduce

This appears to be the offending line ... which also exists in several other components ... which probably emit the same warning:

https://github.com/filamentphp/filament/blob/54a49d9eb85a308702aef96bb5e542c9bef99301/packages/infolists/resources/views/components/entry-wrapper/index.blade.php#L45-L47

Reproduction repository

https://github.com/sorry-none-provided-yet

Additional Notes

While I believe this is a bug, I'm posting it to get feedback about whether you want someone to tackle addressing this, and what strategy to use.

I also observe that some components of Filament allow ->alignment() to be passed with an enum, and others disallow the enum, accepting only string|Closure|null. eg: https://github.com/filamentphp/filament/blob/54a49d9eb85a308702aef96bb5e542c9bef99301/packages/tables/src/Table/Concerns/HasActions.php#L86-L91

I'm wondering if you'd like that addressed in the same PR or kept separate.

[drbyte]

Somewhat related, which might actually fix this, is PR https://github.com/filamentphp/filament/pull/9149

[wychoong]

Somewhat related, which might actually fix this, is PR #9149

don't think that PR will fix this. need to set a default when the alignment is null

[zepfietje]

@drbyte, thanks for reporting. All uses of Enum::tryFrom() should be updated to something like this probably:

if (! $alignment instanceof Alignment) {
    $alignment = filled($alignment) ? (Alignment::tryFrom($alignment) ?? $alignment) : null;
}

Feel free to create a PR for this if you like. 🙂

[drbyte]

@zepfietje - Just reporting on findings while assessing the scope of this.

A search for tryFrom reveals there are 2 different styles used across the project:

This one, only used for iconPosition: if (truthy), do an enum lookup, else null (doesn't passthru the raw value, ever)

    if (! $iconPosition instanceof IconPosition) {
        $iconPosition = $iconPosition ? IconPosition::tryFrom($iconPosition) : null;
    }

This one, used for sizes and alignment: use enum match if found, else passthru the value, whether null or invalid or whatever

    if (! $size instanceof ActionSize) {
        $size = ActionSize::tryFrom($size) ?? $size;
    }

    $iconSize ??= match ($size) {
        ActionSize::ExtraSmall, ActionSize::Small => IconSize::Small,
        default => IconSize::Medium,
    };

    if (! $iconSize instanceof IconSize) {
        $iconSize = IconSize::tryFrom($iconSize) ?? $iconSize;
    }

    if (! $alignment instanceof Alignment) {
        $alignment = Alignment::tryFrom($alignment) ?? $alignment;
    }

    if (! $footerActionsAlignment instanceof Alignment) {
        $footerActionsAlignment = Alignment::tryFrom($footerActionsAlignment) ?? $footerActionsAlignment;
    }

    if (! $verticalAlignment instanceof VerticalAlignment) {
        $verticalAlignment = VerticalAlignment::tryFrom($verticalAlignment) ?? $verticalAlignment;
    }

Your suggested approach's logic is: if (blank/null) use null; else use enum if matched or passthru raw value if not matched

if (! $alignment instanceof Alignment) {
    $alignment = filled($alignment) ? (Alignment::tryFrom($alignment) ?? $alignment) : null;
}

Obviously the way iconPosition does it is slightly different. And we can't change the others to that syntax else we'd probably cause some breaking changes.

But, digging deeper, there are two cases where the component using iconPosition accepts either the enum value or a string ('before' or 'after'), so switching iconPosition to use the other common syntax should break anything ... other than maybe make something "work as coded" where it was mistakenly not doing it before but code hadn't been switched to use an enum.

Anyway, that's the long way of saying: I'm going to proceed with a PR that changes ALL uses of tryFrom to the syntax you proposed.