Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: The component class constructor doesn't seem to run when using ComponentColumn #1513

Closed
nathan-io opened this issue Nov 1, 2023 · 54 comments · Fixed by #1779
Closed
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@nathan-io
Copy link

What happened?

We're trying to use a Blade component as a column:

            ComponentColumn::make('Fine Content', 'fine_weight_ozt')
                ->component('weight-with-conversions')
                ->attributes(fn ($value, $row, Column $column) => [
                    'weight' => new Weight($row->weighing_unit_type, $row->fine_weight),
                ]),

This does cause livewire-tables to attempt to render the component view, but it throws an "Undefined variable" exception because the public properties that are set in the constructor aren't present.

I don't believe the problem is in the component, because it works without issue when called in a view:

@php
     $weight = new App\Data\Weight(1, 1)
@endphp
<x-weight-with-conversions :weight="$weight" />

So perhaps the constructor is not firing?


Here's the component class and view:

...
class WeightWithConversions extends Component
{
    public Weight $weight;
    public string $conversions;

    /**
     * Create a new component instance.
     */
    public function __construct(Weight $weight)
    {
        $this->weight = $weight;

        $this->conversions =
            $weight->toString(WeighingUnitType::TroyOunce) . '<br/>' .
            $weight->toString(WeighingUnitType::Gram) . '<br>' .
            $weight->toString(WeighingUnitType::Pennyweight);
    }

    /**
     * Get the view / contents that represent the component.
     */
    public function render(): View|Closure|string
    {
        return view('components.weight-with-conversions');
    }
}

View:

<button data-popover data-tippy-content="{!! $conversions !!}" class="underline-dotted">
    {{ $weight->toString() }}
</button>

How to reproduce the bug

No response

Package Version

3.1.0

PHP Version

8.1.x

Laravel Version

10

Alpine Version

No response

Theme

None

Notes

No response

Error Message

No response

@nathan-io nathan-io added the bug Something isn't working label Nov 1, 2023
@nathan-io nathan-io changed the title [Bug]: The component class constructor isn't called when using ComponentColumn [Bug]: The component class constructor doesn't seem to run when using ComponentColumn Nov 1, 2023
@lrljoe
Copy link
Collaborator

lrljoe commented Nov 1, 2023

Thanks for raising, I'll have a look in a bit, just so I can confirm:

class WeightWithConversions extends Component

Is that:
Livewire\Component
or
Illuminate\View\Component

I'm assuming the latter?

@nathan-io
Copy link
Author

It's a Blade component.

<?php

namespace App\View\Components;

use Closure;
use Illuminate\Contracts\View\View;
use Illuminate\View\Component;
use App\Data\Weight;
use App\Enums\Item\WeighingUnitType;

class WeightWithConversions extends Component
{
    public Weight $weight;
    public string $conversions;

    /**
     * Create a new component instance.
     */
    public function __construct(Weight $weight)
    {
        $this->weight = $weight;

        $this->conversions =
            $weight->toString(WeighingUnitType::TroyOunce) . '<br/>' .
            $weight->toString(WeighingUnitType::Gram) . '<br>' .
            $weight->toString(WeighingUnitType::Pennyweight);
    }

    /**
     * Get the view / contents that represent the component.
     */
    public function render(): View|Closure|string
    {
        return view('components.weight-with-conversions');
    }
}

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 1, 2023

Perfect, thanks, I'll take a look shortly.

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 1, 2023

Just having a look at why this isn't working for you, as I have a ComponentColumn working in my test environment. Will see if I can figure out why it works for me and not for you!

@nathan-io
Copy link
Author

Thanks for looking into this!

I see one potential issue (weighing_unit_type has a cast), I'll investigate and update this.

@nathan-io
Copy link
Author

nathan-io commented Nov 2, 2023

I looked at it, still no luck. The Weight constructor throws an exception if supplied invalid arguments.

I put a dd() in the component constructor, but I don't get a dump when I view the page with the table. I move the dd() to the component view, and I do.

Can you show me your working test?

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 2, 2023

I'll have to share it tomorrow, as I'm tuckered out now: See Here for why! That should all get merged in, possibly tomorrow as a new release. But then I will definitely share my working example!

Although glancing at it, I think mine may be an anonymous component, which would explain a lot as to why mine is working and yours isn't.

I'll pop the code open tomorrow to see if I can figure out what's going on there.

@nathan-io
Copy link
Author

Although glancing at it, I think mine may be an anonymous component, which would explain a lot as to why mine is working and yours isn't.

Yes I think this is the difference.

Copy link

stale bot commented Dec 17, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added wontfix This will not be worked on and removed wontfix This will not be worked on labels Dec 17, 2023
Copy link

stale bot commented Jan 18, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added wontfix This will not be worked on and removed wontfix This will not be worked on labels Jan 18, 2024
Copy link

stale bot commented Feb 22, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added wontfix This will not be worked on and removed wontfix This will not be worked on labels Feb 22, 2024
Copy link

stale bot commented Mar 30, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Mar 30, 2024
@stale stale bot closed this as completed Apr 13, 2024
@nathan-io
Copy link
Author

This is still an issue

@lrljoe lrljoe reopened this Jun 22, 2024
@stale stale bot removed the wontfix This will not be worked on label Jun 22, 2024
@lrljoe
Copy link
Collaborator

lrljoe commented Jun 22, 2024

Apologies, stalebot gets a bit over-eager (I have no control over that!)

I'll do my best to take a look at this over the next week (so expect an update by the end of the month).

I have tinkered a little, and have got this resolved in my local environment (I've added a new Column type to avoid any conflicts with existing code).

If you want to give it a whirl, ping me on Discord and I'll let you know what to do to give it a test (would appreciate a real-world test tbh)

@lrljoe
Copy link
Collaborator

lrljoe commented Jul 3, 2024

Please do reach out if you want to test this, as otherwise I'll throw it into the next version, and it may not do what you want!

@nathan-io
Copy link
Author

Hello! I don't really use Discord, could you post some code here? I could temporarily modify my local vendor files and test it.

@nathan-io
Copy link
Author

To test this, all you need is a component which has a class constructor that sets some variable. Then reference that variable in the component view to make sure it's available.

@lrljoe
Copy link
Collaborator

lrljoe commented Jul 16, 2024

Okay, replicated, and have a fix for it, it'll come out as a new ViewComponentColumn type.

@lrljoe lrljoe reopened this Aug 4, 2024
@lrljoe
Copy link
Collaborator

lrljoe commented Aug 4, 2024

3.4.0 is out now

This fix will be in 3.4.1 which I'll release momentarily

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 4, 2024

v3.4.1 is out, ViewComponentColumn is the one you want to use, as this has the fix in.

@nathan-io
Copy link
Author

nathan-io commented Aug 4, 2024

Thank you. I upgraded to 3.4.2. I'm getting a different exception now: https://gyazo.com/caed47af06a1744f22e76b8306ce7d14

Usage

            ViewComponentColumn::make('Fine Content', 'fine_weight_ozt')
                ->component('weight-with-conversions')
                ->attributes(fn ($value, $row, Column $column) => [
                    'weight' => new Weight($row->weighing_unit_type->value, $row->fine_weight),
                ]),

Same result with ->component('weightWithConversions')

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 4, 2024

Just to make sure I can replicate it, whats the namespace for your WeightWithConversions component?

@nathan-io
Copy link
Author

It's App\View\Components

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 4, 2024

So I've been able to replicate the issue, and managed to get it working how it should, I now just need to figure out how to implement it properly.

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 4, 2024

Aand, I have a working solution. If you're around today, and fancy giving it a test, I'll create a temporary branch for it, as I'll need update tests/docs etc

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 4, 2024

It's in the development branch at the mo, I've added a new method called customComponent that expects the actual View Component Class to be specified.

This then works as expected (seems to at least so far in my testing!)

            ViewComponentColumn::make('Weight', 'name')
            ->customComponent(\App\View\Components\TestWeight::class)
            ->attributes(fn ($value, $row, Column $column) => [
                'weight' => new Weight(($row->notes_count ?? 10)*rand(5,240))
            ]),

@nathan-io
Copy link
Author

nathan-io commented Aug 4, 2024

When I switch to branch dev-develop (98a5ef3), the ViewComponentColumn class can't be found.

I'll be happy to test this once you've been able to test and release it.

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 4, 2024

@nathan-io - Sorry, the active development branch is:

dev-development

It's looking likely the back end of next week to make it into a release, as there's a fair few tests to write to cater for the different permutations, including adding in some weird/whacky View Components to make sure it operates consistently. Seems to so far tho!

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 8, 2024

@nathan-io - v3.4.3 is out with the fixes in

customComponent

Should you wish to render the Custom Component in it's entirety, then you may use the customComponent method. Otherwise it will pass in the values directly to the blade, rather than executing your View Component.

ViewComponentColumn::make('Weight', 'grams')
    ->customComponent(\App\View\Components\TestWeight::class)
    ->attributes(fn ($value, $row, Column $column) => [
        'weight' => new Weight($value),
    ]),

Docs should get updated in next few days, but that's the new feature that lets you properly instantiate a View Component.

@nathan-io
Copy link
Author

Unfortunately the issue persists with 3.4.3.

            ViewComponentColumn::make('Fine Content', 'fine_weight_ozt')
                ->customComponent(WeightWithConversions::class)
                ->attributes(fn ($value, $row, Column $column) => [
                    'weight' => new Weight($row->weighing_unit_type->value, $row->fine_weight),
                ]),

Result: https://gyazo.com/50dbdc870f5703cbc9612eb8e91d5dd2

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 10, 2024

@nathan-io

In your blade for the component, try

$this->conversions

instead of

$conversions

@nathan-io
Copy link
Author

nathan-io commented Aug 10, 2024

I did try $this->conversions and got exception "Property [$conversions] not found on component: [retail-catalog-item-table]".

However, that isn't the correct way to access the variable from the component view. See https://laravel.com/docs/11.x/blade#passing-data-to-components:

You should define all of the component's data attributes in its class constructor. All public properties on a component will automatically be made available to the component's view. It is not necessary to pass the data to the view from the component's render method:
...
When your component is rendered, you may display the contents of your component's public variables by echoing the variables by name:

<div class="alert alert-{{ $type }}">
    {{ $message }}
</div>

While reviewing the docs, I noticed that my syntax was a bit different. So I refactored to this:

class WeightWithConversions extends Component
{
    public function __construct(
        public Weight $weight,
        public string $conversions = ''
    )
    {
        $this->conversions =
            $weight->toString(WeighingUnitType::TroyOunce) . '<br/>' .
            $weight->toString(WeighingUnitType::Gram) . '<br>' .
            $weight->toString(WeighingUnitType::Pennyweight);
    }

    public function render(): View|Closure|string
    {
        return view('components.weight-with-conversions');
    }
}

This works exactly the same as my previous implementation - it's fine when called like <x-weight-with-conversions :weight="$weight"/> within a Blade view: https://i.gyazo.com/a968845f7632fe8f2c994a1a8abb0818.png

However, when using this in the table column, I still get the "Undefined variable $conversions" exception in the component view.

            ViewComponentColumn::make('Fine Content', 'fine_weight_ozt')
                ->customComponent(WeightWithConversions::class)
                ->attributes(fn ($value, $row, Column $column) => [
                    'weight' => new Weight($row->weighing_unit_type->value, $row->fine_weight),
                ]),

I reverted my changes to WeightWithConversions and am still getting the same result. It works fine when called as <x-weight-with-conversions>, but throws "Undefined variable $conversions" when used as a ViewComponentColumn.

"

class WeightWithConversions extends Component
{
    public Weight $weight;
    public string $conversions;

    public function __construct(Weight $weight)
    {
        $this->weight = $weight;

        $this->conversions =
            $weight->toString(WeighingUnitType::TroyOunce) . '<br/>' .
            $weight->toString(WeighingUnitType::Gram) . '<br>' .
            $weight->toString(WeighingUnitType::Pennyweight);
    }

    public function render(): View|Closure|string
    {
        return view('components.weight-with-conversions');
    }
}

Do you have an example component class and view that you were able to get working with this? If so, I can try it out.

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 14, 2024

I will put together an example later today, and triple check that it is working as it should

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 16, 2024

So after mucking around for quite a while, the methods within construct() aren't being fired, that's applying both internally to the table, and externally.

The way I've got it working is:

            ViewComponentColumn::make('Test','amount')
            ->customComponent(\App\View\Components\TestWeight::class)
            ->attributes(fn ($value, $row, Column $column) => [
                'weight' => new \App\Weight($row->amount),
            ]),

The View Component "TestWeight"

<?php

namespace App\View\Components;

class TestWeight extends \Illuminate\View\Component
{
    public ?\App\Weight $weight;

    public function __construct(\App\Weight $weight)
    {
        $this->weight = $weight;
    }

    /**
     * Get the view / contents that represent the component.
     */
    public function render(): \Illuminate\Contracts\View\View|Closure|string
    {
        return view('test-weight')->with(['conversions' => $this->weight->getConversion()]);
    }

}

Then the test-weight.blade.php blade:

    <div>
        {{ $conversions }}
    </div>

Not ideal, but not sure why it's not firing subsequent code within construct as of yet, I'm assuming it's something I'm missing in terms of Reflections

@nathan-io
Copy link
Author

nathan-io commented Aug 18, 2024

Thank you for the follow up and all of your efforts in this.

Not ideal, but not sure why it's not firing subsequent code within construct as of yet, I'm assuming it's something I'm missing in terms of Reflections

Yes I've always suspected that the constructor was either not firing at all, or somehow the data wasn't making it to the view the way it normally would when we call the component within a standard Blade context.

With that in mind, I tried a workaround that I honestly should've though of from the beginning:

use Illuminate\Support\Facades\Blade;

// ...

Column::make('Fine Content', 'fine_weight_ozt')
    ->format(fn($value, $row, Column $column) => Blade::render('<x-weight-with-conversions :weight="$weight"/>', 
        ['weight' => new Weight($row->weighing_unit_type->value, $row->fine_weight)])
    )
    ->html(),

Result: (screenshot)

It also works using view(), I just didn't want to create another file.

Column::make('Fine Content', 'fine_weight_ozt')
    ->format(
        fn($value, $row, Column $column) => view('weight-with-conversions-container')->with([
            'weight' => new Weight($row->weighing_unit_type->value, $row->fine_weight)
        ])
    ),

That weight-with-conversions-container view was just:

<x-weight-with-conversions :weight="$weight"/>

Someone could use this approach to use Blade components or Livewire components (which may suffer from the same or similar issue) as columns without having to touch the component class or view.

The only issue I've noticed is that when I paginate, the links stop toggling the tippy popup. I'm such that's just some state thing.

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 21, 2024

I'm assuming you're using bootstrap?

@nathan-io
Copy link
Author

Tailwind

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 21, 2024

In that case, 100% look at the native AlpineJS anchor approach

https://alpinejs.dev/plugins/anchor

Most of the 3rd party popover/popper packages only run on first load, and setup instances for any element with "xyz" attributes.

@nathan-io
Copy link
Author

nathan-io commented Aug 22, 2024

Thanks, I'll check that out!

As far as this issue, I wonder if ViewComponentColumn could be simplified based on this approach?

Instead of passing in a class name or arguments, you just pass in a string with the Blade to render, and the values for any props you're passing. In this case for example:

ViewComponentColumn::make('Fine Content', 'fine_weight_ozt')
    ->bladeComponentTag('<x-weight-with-conversions :weight="$weight"/>')
    ->attributes(fn ($value, $row, Column $column) => [
        'weight' => new Weight($row->weighing_unit_type->value, $row->fine_weight),
    ]),

Then in ViewComponentColumn you could replace:

        if ($this->hasCustomComponent()) {
            $reflectionClass = new ReflectionClass($this->getCustomComponent());

            $reflectionInstance = $reflectionClass->newInstanceArgs($attributes);

            return $reflectionInstance->render();
        } else {
            return view($this->getComponentView())->with($attributes);
        }

with something like:

return Blade::render($bladeComponentTag, $attributes);

Not sure passing a Blade component tag rather than a class name feels 100% right, but at least it would** bypass the issue with the constructor.

**I think? maybe there's something special about doing it within the Livewire table class itself via a closure passed to Column::format() versus doing it from another class like ViewComponentColumn or LivewireComponentColumn

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 24, 2024

@nathan-io

With fresh eyes, and considering what you shared, the simplest approach would really be a simple Custom View against a standard Column:

Create a Column that uses a Custom View.
E.g:
"test-weight-column"

That recieves a parameter of "$weight"

            Column::make('NewWeight', 'amount')
            ->format(
                fn($value, $row, Column $column) => view('test-weight-custom-column-view')->withWeight(new \App\Weight($row->amount))
            ),

Create the Custom Blade: "test-weight-custom-column-view.blade.php"

<div>
    <x-test-weight :$weight />
</div>

This will then instantiate and call the "Test Weight" view Component:

namespace App\View\Components;

class TestWeight extends \Illuminate\View\Component
{
    public ?\App\Weight $weight;

    public $conversions;

    public function __construct(\App\Weight $weight)
    {
        $this->weight = $weight;
        $this->conversions = $weight->getConversion();
    }

    public function render(): \Illuminate\Contracts\View\View|Closure|string
    {
        return view('test-weight-123');
    }
}

Then the "test-weight-123" blade contains something like:

<div>
        Conversions: {{ $conversions }}
</div>

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 24, 2024

@nathan-io

I've been fiddling a little with the existing methods, and this seems like it may work for what you want:

            Column::make('Test Weight', 'amount')
            ->format(
                fn($value, $row, Column $column) => Blade::render("<x-test-weight :weight='new \App\Weight($row->amount)' />")
            )->html(),

TestWeight is a VIew Component, code below

namespace App\View\Components;

class TestWeight extends \Illuminate\View\Component
{
    public ?\App\Weight $weight;

    public $conversions;

    public function __construct(\App\Weight $weight)
    {
        $this->weight = $weight;
        $this->conversions = $weight->getConversion();
    }

    /**
     * Get the view / contents that represent the component.
     */
    public function render(): \Illuminate\Contracts\View\View|Closure|string
    {
        return view('test-weight-123');
    }

}

Which has a view of:

    <div>
        {{ $conversions }}
    </div>

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 27, 2024

Thanks, I'll check that out!

As far as this issue, I wonder if ViewComponentColumn could be simplified based on this approach?

Yep, I'll certainly look at options for simplifying it based on the approach I highlighted above. It won't be in the next 2 versions, but may be in a subsequent one.

@lrljoe lrljoe added enhancement New feature or request and removed bug Something isn't working labels Aug 27, 2024
Copy link

stale bot commented Sep 29, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Sep 29, 2024
@stale stale bot closed this as completed Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants