Help Me Solve The Hardest Problem In Livewire (And Maybe The Entire World)

Tbh i do get not wanting to leak information to the front end, but the raw fact is alot of the time database ids are being passed around which have far more security implications than a class name.

… but …

If it can be abstracted it satisfies those who feel uneasy about it, AND prevents breaking changes during iteration should your models get relocated in future.

1 Like

Is the “need” for this only for type hinted models in the mount method?

If so i’m sure some reflection could do all of this without config required?

given:

public function mount(Post $post, Foo $bar)
    {
        $this->post = $post;
        $this->bar = $bar;
    }

You can work out the class and the property name programatically. The only thing to consider here is users would need to be aware the param names MUST match class property name exactly.

1 Like

I know we are taking the more simple issue here first, which is a model, but let’s not forget about relationships as well. What if I want $comments?

Should there be support for something like this?

protected $models = ['post.comments'];

Where every time the Livewire component is hydrated, you get the related comments as well? Should there be some way to determine if you want the comments to update on every request or only on specific ones?

Sorry I don’t have more answers, just lots of questions. :sweat_smile:

Very good point. i think creating a config structure for all these type of cases could get pretty confusing.

in the concept ive put forward maybe the __get call to rehydrate the model should first look for a hydratePostModel method before using the default logic of model::find.

And then if needed it can fetch related models.

/**
 * Optionally define a `hydrate{name}Model` to fetch the model and 
 * perform any other operations, such as loading related models.
 */
public function hydratePostModel($id)
{
    // the default behaviour: return Post::findOrFail($id);
    return Post::with('comments')->findOrFail($id);
}

All good points.

I really want this API to feel simple, clean and intuitive. Also, I want to offer a flexible alternative for more control.

Here’s the API currently rattling around in my head:

class ShowPost extends Component
{
    public $post;

    protected $casts = ['post' => 'model'];

    public function mount(Post $post)
    {
        $this->post = $post;
    }
}

Internally Livewire will “dehydrate” the model to it’s class, and id. (I still have questions about what this will look like in the payload).

It WONT automatically re-hydrate it, until $this->post is called in the component (I’ll work some PHP magic to make this happen). At which time it will use “Post::find($id)” to hydrate it.

If the user wants anything more (hiding the full model class, adding eager loads), we can do one of two things:

#1: Allow custom “casters” (PHP classes that offer a hydrate and dehydrate method). For example:

protected $casts = ['post' => PostCaster::class];

...

class PostCaster implements LivewireCaster
{
    public function hydrate($value) {}
    public function dehydrate($value) {}
}

#2: Just use the “computed property” api I proposed earlier. (doesn’t exist yet)

class ShowPost extends Component
{
    pubic $postId;

    public function mount(Post $post)
    {
        $this->postId = $post->id;
    }

    public function getPostAttribute()
    {
        return Post::find($this->postId);
    }
}

So, I suppose my question is:
A) Is the proposed default strategy (store the class and id, and rehydrate using ::find) a common enough path to cover the average use case? (I’m fine with just warning them about what’s happening in the docs (exposing the class namespace))

I like the api.

Personally I would see the hydrate api similar to model attribute modification hence the hydrateAttributeModel method idiom. But external hydrators is a nice separation and probably good for reuse in other components.

I’m curious how your going to implement setting and lazy access if the developer has declared the public variable on the class.

Cool, yeah I’m going to do some hacky stuff, but basically: unset($component->post) will allow a call to $component->post to hit the __get() magic method. Not the most elegant solution, but the API is worth it to me.

2 Likes

I like the computed property (option 2) because that’s cleaner to me. If I want to add eager loading or anything else, this feels very intuitive to me.

Option 1 with Casting, hydrating, and dehydrating is going to confuse a lot of people who don’t understand the full Livewire lifecycle IMO. It’s not immediately clear to me how to use that PostCaster, where the first API is very clear.

Good insight Kevin,

I think I’m coming around to that line of thinking. I think I just may make “computedProperties” a thing for this new tagged release and see if that solves our woes.

Now, the question is: what is the naming convention of computed property methods:

A) ->computedFoo()
B) ->getFooAttribute()
C) ->getFooProperty()

Here are the pros to each of them:
A) Feels the most intuitive to me. Only one extra word. Is the shortest.
B) Leverages the existing knowledge of Eloquent Model accessors. I like not adding extra API to remember.
C) “getXAttribute” is kinda incorrect, because Livewire calls them “properties” not “attributes” like an Eloquent model. So this would leverage the pattern of Eloquent, but adapt it to a Livewire context.

Here’s my order or preference currently: A, C, B

Thoughts anyone? (on the decision to go with this as well.)

I personally prefer A or C. I like C better myself because again, it has the “Laravel” feel of an accessor, but A is very similar to the lifecycle hooks convention of updatedFoo() so should be natural for people who are comfortable with Livewire. I like either.

Makes sense.

The next problem:

It would be great if computed properties were automatically available to the Blade view as regular variables (like public properties are):

<div>
    {{ $post }} // is evaluated from "->computedFoo()"
</div>

Accomplishing this wouldn’t be all that hard. The problem though, is one of the virtues of these “computed” properties is only being evaluated when they are needed (this way if you don’t use the model on every request, the db query won’t be executed when it’s not needed).

Unfortunately, there is no “with” statement like there is in JavaScript that would magically allow us to resolve undefined variables at run-time (like a magic __get method for global variables).

Which brings me to the next logical API for computed properties:

<div>
    {{ $this->post }}
</div>

Access to the $this object from a view.

This seems fine to me, although NOW I’m bummed that the user has to know when to use $this->foo and just $foo based on whether or not the property is a public property OR a computed property.

Which honestly, is making me feel like $this->property should be the idiomatic way to access public properties, and we should forget the whole “automatically pass public props to the blade view”. There’s already been requests for access to $this inside the Blade view to call methods, this would make things unified, and actually would feel quite nice.

I think this is what I’m leaning towards… ugh… thoughts?

Update: I made all these updates we’ve talked about.

You can try everything out on dev-master (and please do to vet it for bugs)

Updates made:

  • Added Computed Properties (will be the idiomatic way to access eloquent models)
  • Added Casters for carbon dates, collections, and support for custom caster classes (was going to add these anyways) (will NOT be the idiomatic way to access models)
  • Stopped passing public properties through to the blade view automatically, accessing properties with $this->property WILL be the idiomatic way.

What do we think? Am I crazy? I finally feel pretty good about all this.

Thanks everyone SOOOO much for the help fleshing this out.

1 Like

Really interesting and well explained thread everyone!

Caleb, are you proposing removing protected (cached) properties entirely with the addition of the computed properties? I like the current cached property for its ability to hold a modified state in memory (Cache) before later committing the changes to the Database model but while also preventing that data from being passed back and forth to the client.

Wouldn’t the proposed approaches loose that ability (of having a dirty, in progress model instance that does not get passed to the browser in its entirety)?

This does remove cached protected properties and would lose the ability you describe. However, you should still be able to do your own caching if you need it. I believe Caleb was going to work on some documentation outlining some best practices / recommendations for implementing your own cache.

I think the loss of the protected/cached properties is a pretty big loss. After thinking about this a bit more however, I was also wondering if the new approaches solve the issue of the counters reverting to 1&1 on the back button. From my understanding, that will not be solved.

On the other hand the counters going to 2&3 on thew subsequent increment seems correct from a certain point of view. The public one goes to 2 because 2 was not saved to any persistent storage previously. The protected goes to 3 (which is correct in terms of number of times it was incremented) because its actual values was persisted in the cache.

So I would respectfully introduce the theory that perhaps this not an actual bug and just an inconvenience caused by the back button scenario. But unless the back button scenario (which is already an edge case) could be resolved, maybe this is not an issue.

Again, don’t mean to stir the pot, just wanted to share my point of view. Curious to hear what you guys think…

I wanted to test version 0.5.0 out on my app, but after changing my composer.json to
“livewire/livewire”: “dev-master@dev” or “livewire/livewire”: “^0.5.0”
I get an error:

Illuminate\Contracts\Container\BindingResolutionException : Target class [blade.compiler] does not exist.
at /app/vendor/laravel/framework/src/Illuminate/Container/Container.php:805

What am I doing wrong?

Hmm, in another app (yes, I’m using Livewire in three different apps already) I could update to 0.5.0 without errors. Strange…

Hey Erik, please submit an issue on the GitHub repo for this. I’d lack to track them there.

@csti, I love the input - it’s not pot stirring.

Good points here. I’ve been thinking about it more over the weekend, and although I agree protected properties is a big loss, I really think the project is better off. It does suck to make Livewire less user-friendly and “invisible”, but I will sleep more soundly at night knowing it’s not juggling 2 completely different state paradigms. I also just have the feeling that we will keep playing weird-edge-case whack-a-mole because of it.

Again, tough one, but I’m feeling more sure of myself on this as I sit with it more.

Ok, the saga continues. Dries is suggesting we rethink this decision and add support for Eloquent models using the “SerializesModels” trait from Queueables.

Here’s the thread on GH: https://github.com/livewire/livewire/issues/488

Input appreciated!

Ok, some decisions have been made.

  • Still don’t want to cache protected properties like before
  • Instead we will “serialize” eloquent models and “unserialize” them between Livewire requests.

You can now set public properties as models or model collections

2 Likes