Skip to content

Model.$with#45

Open
dan-gamble wants to merge 7 commits intojplhomer:mainfrom
dan-gamble:model-with
Open

Model.$with#45
dan-gamble wants to merge 7 commits intojplhomer:mainfrom
dan-gamble:model-with

Conversation

@dan-gamble
Copy link
Contributor

@dan-gamble dan-gamble commented Apr 25, 2023

This is in response to #36

First PR so please bear with me @jplhomer!

This feels like it's been way "simpler" than I was expecting so I feel I'm almost certainly missing something.

The doc wording has been "lifted" from Laravel's docs as I'm really not that great at writing so I felt that was a good place to start. I've not used Laravel but I used Laravel's implementation as inspiration for this.

TL;DR static $with

export class User extends Model {
  static $with = ["posts", "profile"];

  posts?: Post[] | Promise<Post[]>;
  $posts() {
    return this.hasMany(Post);
  }

  profile?: Profile | Promise<Profile>;
  $profile() {
    return this.hasOne(Profile);
  }
}

@dan-gamble dan-gamble changed the title Model.with Model.$with Apr 25, 2023
@dan-gamble dan-gamble marked this pull request as ready for review April 25, 2023 19:01
@dan-gamble
Copy link
Contributor Author

I think this is ready now @jplhomer

Copy link
Owner

@jplhomer jplhomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Thank you. A couple questions:

  • What's your thinking behind $with instead of with? I know the convention is kind of there with the double-relation name thing already, but is it useful to perpetuate to all other sorts of conventions?
  • Does it need to be static? I noticed Laravel's is an instance method instead of a static method.

@dan-gamble
Copy link
Contributor Author

Does it need to be static? I noticed Laravel's is an instance method instead of a static method.

Nope! I swear I was getting a syntax error in my editor when doing this but I don't seem to now. That explains why I went static with it. With that said, because this is at a class level as opposed to an instance level (compared to a relationship accessor) I feel static might make sense still. I'm easy either way.

What's your thinking behind $with instead of with? I know the convention is kind of there with the double-relation name thing already, but is it useful to perpetuate to all other sorts of conventions?

For me I took the $ as evidencing something "magic". I wanted to preserve all static attributes so they can be mapped to the database. A column of with for example would overwrite this or we'd need to provide an explanation in the Proxy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants