tobyink / p5-sub-handlesvia

Perl 5 distribution Sub-HandlesVia; see homepage for downloads and documentation.
https://metacpan.org/release/Sub-HandlesVia
3 stars 4 forks source link

Doesn't play well with Mouse when other traits are used #3

Closed bbrtj closed 2 years ago

bbrtj commented 2 years ago

I use a couple of traits in my Mouse code. When I try to import Sub::HandlesVia normally, I get a strange error:

Due to a method name conflict in roles 'My::Moose::Trait::AutoSetters', 'My::Moose::Trait::FakeRequired', and 'My::Moose::Trait::Serializable', the method 'has' must be implemented or excluded by 'Mouse::Meta::Class::__ANON__::1' 
at /usr/home/devenv/project/local/lib/perl5/amd64-freebsd/Mouse/Meta/Role/Composite.pm line 136

I figured out the problem may be using Role::Tiny to apply traits, so I tried to apply the trait manually by extending the import list of Mouse. No luck:

You can only consume roles, Sub::HandlesVia::Toolkit::Mouse::PackageTrait is not a Mouse role 
at /usr/home/devenv/project/local/lib/perl5/amd64-freebsd/Mouse/Util.pm line 336.

Role::Tiny is not compatible with Mouse roles, so the entire thing falls apart with a Mouse system complex enough. To fix this, I reimplemented Sub::HandlesVia::Toolkit::Mouse::PackageTrait locally. The only change I did was to replace use Role::Tiny with use Mouse::Role - and now it works flawlessly, when using -traits => ['My::Sub::HandlesVia::Toolkit::Mouse::PackageTrait']

Not sure how to fix this on Sub::HandlesVia level, without depending on Mouse itself.

tobyink commented 2 years ago

Using Mouse::Role instead of Role::Tiny within lib/Sub/HandlesVia/Toolkit/Mouse.pm should be safe as that file is only loaded when Mouse has already been detected.

Are you able to provide an example script which currently fails though?

bbrtj commented 2 years ago

It's hard to reproduce the exact same error message I got in my private project, but it's quite easy to produce any error when using traits:

error.pl

use v5.36;
use lib '.';

package Parent {
    use v5.36;
    use Mouse -traits => [qw(
        My::Trait::AutoSetters
    )];
    # use Mouse;
    use Sub::HandlesVia;

}

package ThisFails {
    use v5.36;
    use Mouse;
    use Sub::HandlesVia;

    extends 'Parent';

    has test => (
        is => 'ro',
        default => sub { [] },
        handles_via => 'Array',
        handles => {
            'add_test' => 'push...'
        }
    );
}

my $t = ThisFails->new;
$t->set_test([3]);
$t->add_test(5)->add_test(6)->add_test(7);

use Data::Dumper; die Dumper($t->test);

My/Trait/AutoSetters.pm

package My::Trait::AutoSetters;

use v5.36;
use Mouse::Role;

around add_attribute => sub {
    my ($orig, $self, $name, @args) = @_;
    my %params = @args == 1 ? $args[0]->%* : @args;

    if (exists $params{writer} && !$params{writer}) {
        delete $params{writer};
        return $self->$orig($name, %params);
    }

    # exit early if it's not something we want or can alter
    return $self->$orig($name, @args)
        if $name =~ /^_/
        || $name =~ /^\+/;

    $params{writer} //= "set_$name";

    my $attribute = $self->$orig($name, %params);

    return $attribute;
};

1;

running perl error.pl produces this error:

Mouse::Meta::Class cannot have Mouse::Meta::Class::__ANON__::1__WITH__Sub::HandlesVia::Toolkit::Mouse::PackageTrait as a super class because of their metaclass incompatibility at /home/devenv/.carmel/5.36.0-amd64-freebsd/builds/Mouse-v2.5.10/blib/lib/Mouse/Meta/Class.pm line 154.

If you replace use Mouse -traits with just a regular one, it compiles ok, but quickly dies due to unknown set_test method.

Note: seems like My::Trait::AutoSetters has to be in its own file to work properly with Mouse.

This example can be fixed with repeating the exact same trait list in ThisFails class - no more, no less, so that it ends up being the same ANON class of Mouse, with the same Role::Tiny roles.

bbrtj commented 2 years ago

If you do s/Mouse/Moose/g in my example above, it will fail as well with a similar error message:

The metaclass of ThisFails (Moose::Meta::Class__WITH__Sub::HandlesVia::Toolkit::Moose::PackageTrait) is not compatible with the metaclass of its superclass, Parent (Moose::Meta::Class::__ANON__::SERIAL::1__WITH__Sub::HandlesVia::Toolkit::Moose::PackageTrait) at /home/devenv/.carmel/5.36.0-amd64-freebsd/builds/Moose-2.2201/blib/lib/Moose/Exporter.pm line 418
tobyink commented 2 years ago

Looks like I already played around with this idea a while ago, but couldn't get it working:

https://github.com/tobyink/p5-sub-handlesvia/commit/0635476fcf1b2ed49e5f59abdd129dab95d1df7f

Anyway, it should work now. I'll release 0.039 to CPAN today.