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

Serializing Lucid Models #64

Open
solomonhawk opened this issue Aug 6, 2024 · 2 comments
Open

Serializing Lucid Models #64

solomonhawk opened this issue Aug 6, 2024 · 2 comments

Comments

@solomonhawk
Copy link

solomonhawk commented Aug 6, 2024

Hello 👋

Thank you for all the work you've invested in putting this together. I have a question that perhaps isn't ultimately specific to the Adonis+Remix combo (moreso for lucid + client-rendered framework), but certainly rears it's head prominently once you get into returning models from loaders. I hope by posting it here other folks who run into the same thing might find the discussion useful.

Lucid models aren't type-safe. In practice, what this means is that when you query for a model in lucid and return it from a loader, remix picks up the type that lucid defines as a model's toJSON() method (which is just ModelObject, aka { [key: string]: any }):

// post_service.ts
class PostService {
  // typescript infers this as `(id: number) => Promise<Post>`
  async get(id: number) {
    return Post.getOrFail(id)
  }
}
// (some remix route)
export async function loader({ context, params }: LoaderFunctionArgs) {
  const postService = await context.make('post_service')

  return json({
    post: postService.get(Number(params.id))
  })
}

export default function PostShow() {
  // typescript infers this as `JsonifyObject<{ post: Post }>` which simplifies to `{ post: ModelObject }` due to lucid's typings
  const { post } = useLoaderData<typeof loader>()

  // ..
}

It may be tempting to just type-cast the useLoaderData hook return value as { post: Post }:

const { post } = useLoaderData() as { post: Post }

But that's not a great idea for a few reasons:

  • typescript won't be able to catch problems if we change what gets returned from the loader, since we're overriding the type
  • an instance of the Post model class has methods on it that the serialized data will not have
  • it's cumbersome and duplicative to write these types out every time
  • the type may not include additional relations or extra fields we've added from the lucid query
  • non-serializable fields like Date/DateTime/Regex/BigInt, etc. won't have the correct type (since they will have been JSON.stringified)

It seems sensible then to have some serialization logic somewhere that turns the result of a particular lucid query into a proper plain object. In that case, where's the best place to put that logic?

We could treat the service layer as a boundary, encapsulating the concern of using the ORM and then serializing the resulting data into plain objects. However, it may be beneficial to allow adonis service methods to return proper model class instances which callers can then operate on (in other adonis code).

My current thought is to manually handle serialization at the network boundary (i.e. in the loader, before returning the data). It's tricky to do right though since a service method that returns a Post may have preloaded certain associations or have annotated the resulting rows with additional properties (i.e. $extras) - i.e. any of the things lucid allows you to do when querying (which are mostly things that knex allows you to do).

As the author of this remix-adonisjs, I'm curious to get your thoughts on the matter.

Cheers 👋

@jarle
Copy link
Owner

jarle commented Aug 8, 2024

This is an interesting problem that I have encountered myself when building.

I ususally peek at the work done in the AdonisJS Inertia implementation to see how they handle these cases, and I see that they suggest casting the model to a simple object or adding DTO classes: https://docs.adonisjs.com/guides/views-and-templates/inertia#model-serialization

I personally create inlined DTOs where I extract the fields I need like this:

json({
  title: post.title,
  description: post.description,
})

A more elegant solution would be nice for this. Maybe it could be possible to augment the types for the Remix json method to give better typing with something like this that infers the model fields:

type ExtractModelFields<T> = {
  [K in keyof T as T[K] extends Function ? never : K extends `$${string}` ? never : K]: T[K]
}

It doesn't handle relationships. but might be a start :)

@solomonhawk
Copy link
Author

Oh yeah the inertia serialization docs are informative. Between casting the model.serialize() and explicit DTO modeling I think I lean toward the more explicit pattern.

Manually serializing the objects on a per-case basis in loaders is clean and simple, but doesn't scale that well if you have many loaders and you're doing the same serialization over and over.

Depending on the use case I can see there being many different ways you might end up serializing a model so I shy away from the idea of a generalized type that extracts the model attributes. I also think it might be tricky to augment the types since json(..) makes use of Jsonify<T> under the hood and that pulls a type from an objects toJSON() method if it's present. It's certainly possible to monkey-patch something in there but it's not without some risk.

I ended up just defining presenters for each model+use case combination. What I don't like about it is that you still are in a situation where you need to know that PostWithAuthorDTO has to be used to serialize the data from PostService.getWithAuthor(..) (i.e. there's a coupling between the specific DTO and the service methods that I don't want to enforce in the service layer (again I think it's useful to just return real lucid model objects there for use in other parts of the adonis codebase).

I'll keep working with it and if I come up with something I really like I'm happy to share it back here.

Thank you for sharing your thoughts!

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

No branches or pull requests

2 participants